Page MenuHomeFreeBSD

Fix CID 1307758: Unused value.
AbandonedPublic

Authored by araujo on Jun 19 2015, 10:21 PM.

Details

Summary

The logic here is weird, seems delim should be set before the printf.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

araujo retitled this revision from to Fix CID 1307758: Unused value..
araujo updated this object.
araujo edited the test plan for this revision. (Show Details)
araujo added reviewers: neel, rodrigc, grehan.
  • It still doesn't looks right, I remake the patch and remove the '\0'.
  • Also initialize the variable map_size, CLANG complains about it.
  • Fix a minor style, an space at variable gpa.

Thanks for the quick turnaround, but this idiom is intentional (I updated the CID a couple of hours ago).

The idea is that when pretty-printing the 'flags' value the first one should not have any delimiter (hence 'delim' is initialized to '\0'). However, any subsequent fields must be delimited by a '/' to distinguish them from the previous field.

It is technically correct that the last field that is pretty-printed does not need to set 'delim = /'. However, these blocks of code are usually copy-pasted so when more fields are added in the future this will continue to work correctly.

usr.sbin/bhyvectl/bhyvectl.c
464

clang is confused but if you want to fix this then is to move the initialization of 'map_size' before the first 'goto done'.

1561

This is wrong.

'delim' should be '\0' until the first field is pretty-printed. After that is should be set to the real delimited '/'.

1572

Huh? There *should* be a space before and after the '+=' operator.

araujo edited edge metadata.

@neel thanks for the review.
Here is the correct patch then. Although I think, it is not worth
to commit this patch.

Tks.

usr.sbin/bhyvectl/bhyvectl.c
493

This is introducing a bug in the common case where 'map_size' is getting reset to 0. Yes, this is not worth committing :-)

What I meant was to do this instead:

error = -1;
bitmap = MAP_FAILED;
if (cpu_intel)
        map_size = PAGE_SIZE;
else
        map_size = 2 * PAGE_SIZE;

fd = open("/dev/mem", O_RDONLY, 0);
if (fd < 0) {
        perror("Couldn't open /dev/mem");
        goto done;
}

Not worth to waste time on it, sorry guys.