Page MenuHomeFreeBSD

Fix CID 1307758: Unused value.
AbandonedPublic

Authored by araujo on Jun 19 2015, 10:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Feb 19, 5:36 PM
Unknown Object (File)
Jan 31 2024, 7:11 AM
Unknown Object (File)
Dec 26 2023, 6:42 AM
Unknown Object (File)
Nov 4 2023, 1:17 PM
Unknown Object (File)
Sep 28 2023, 5:01 AM
Unknown Object (File)
Sep 16 2023, 12:42 AM
Unknown Object (File)
Sep 10 2023, 6:28 AM
Unknown Object (File)
Sep 3 2023, 9:44 PM
Subscribers

Details

Summary

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

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No 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.