Add missing va_end's after corresponding va_start's to cleanup state
Do some minor style(9) clean up in affected functions.
MFC after: 3 days
CID: 1018500-1018503
Reported by: Coverity
Sponsored by: EMC / Isilon Storage Division
Details
Details
- Reviewers
cem
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 3490 Build 3530: arc lint + arc unit
Event Timeline
| lib/libcam/scsi_cmdparse.c | ||
|---|---|---|
| 694–695 | Agreed. | |
Comment Actions
- Remove superfluous va_end I added in the previous diff.
- Replace va_list with va_list* in encode functions so we can use a more portable idiom for omitting NULL than bzero'ing va_list.
- Add an assert to ensure that either the getter/putter function or the va_list is provided.
Comment Actions
I'm fine with using (va_list *)NULL as a sentinel value here.
I'm less excited about adding assert(3), just because assert(3) is usually kind of useless.
| lib/libcam/scsi_cmdparse.c | ||
|---|---|---|
| 120–121 | 99% of the time assert(3) will just compile out in userspace. Does this library have any other use of assert(3) (i.e. does it expect anybody to actually compile with -DDEBUG or whatever)? | |
| 134 | Wowza. | |
Comment Actions
Remove asserts; they're inconsistent and will likely be compiled out, as noted by cem.
TODO: fix up APIs to note requirements and be more graceful when provided bad input
| lib/libcam/scsi_cmdparse.c | ||
|---|---|---|
| 120–121 | assert(3) is opt-out. You need to compile with -DNDEBUG to disable them. We only do that in bsd.{lib,prog}.mk if WITHOUT_ASSERT_DEBUG is set. So they're on by default AFAIK. | |
| lib/libcam/scsi_cmdparse.c | ||
|---|---|---|
| 120–121 | For some (incorrect) reason I thought -DNDEBUG got inserted automatically at -O1 and higher. It looks like that is not the case for compilers. However, toolchains (autoconf, CMake, FreeBSD build process) may insert the flag automatically for optimized builds. | |