PVS Studio reports: /usr/src/sys/geom/geom_slice.c:339:1: error: V595 The 'pp' pointer was utilized before it was verified against nullptr. Check lines: 339, 342. > if (indent == NULL) { > sbuf_printf(sb, " i %u", pp->index); > sbuf_printf(sb, " o %ju", > (uintmax_t)gsp->slices[pp->index].offset); > return; > } > if (pp != NULL) { > sbuf_printf(sb, "%s<index>%u</index>\n", indent, pp->index); > ...
@Alexey Could you (for this class of issue being reported), run svn blame's on the relevant files and CC committers of those sections of code and/or relevant major recent committers to the files? That'll save me the time doing that, and I don't want to miss any of these issues (and I might). Thanks for reporting these issues. Does PVS make its output available in a report format we could share with developers more broadly? I understand it includes false positives, but I imagine it would be a good resource for many to look at nontheless.
Sometimes I do "svn blame" (as e.g. in bug #238167), sorry I didn't add it this time, here it is: > 106101 phk if (indent == NULL) { > 106101 phk sbuf_printf(sb, " i %u", pp->index); > 106101 phk sbuf_printf(sb, " o %ju", > 106101 phk (uintmax_t)gsp->slices[pp->index].offset); > 106101 phk return; > 106101 phk } When commit is fairly recent (this one is not), I'd usually reply to the committer directly (via the original commit mail). > I don't want to miss any of these issues (and I might). I think you've missed bug #238194. :-) > Does PVS make its output available in a report format we could share > with developers more broadly? PVS is not doing these runs, I'm doing them myself. I've posted a few messages to svn-src-head@ a week ago, with a link to a full log, you can find it in my $HOME at freefall (pvs-kernel-filtered-2019-05-28.log.xz). > I understand it includes false positives, but I imagine it would be > a good resource for many to look at nontheless. I hope so; I do keep a list of false positives that people send me to reduce the noise in subsequent runs.
dumpconf() is an overloaded API. It is called in multiple ways in geom_dump.c: 1. NULL indent, NULL cp, !NULL pp 2. !NULL indent, NULL cp, !NULL pp 3. !NULL indent, NULL cp, NULL pp I.e., NULL indent => !NULL pp. But !NULL indent implies nothing about pp. The logic in g_slice_dumpconf is correct, though confusing. It would probably be more clear to assert these invariants, which might inform PVS-Studio well enough to clear the false positives. Something like: #define KASSERT_IMPLIES(a, b, c) KASSERT(!(a) || (b), c) ... KASSERT_IMPLIES(indent == NULL, pp != NULL, ("dumpconf API violation")); tl;dr false positive