On Wed, Aug 3, 2022 at 12:15 PM Richard W.M. Jones <rjones@xxxxxxxxxx> wrote: > If it helps to think about this, Coverity checks for consistency. > Across the whole code base, is the return value of a function used or > ignored consistently. You will see Coverity errors like: > > Error: CHECKED_RETURN (CWE-252): [#def37] > libnbd-1.12.5/fuse/operations.c:180: check_return: Calling "nbd_poll" without checking return value (as is done elsewhere 5 out of 6 times). > libnbd-1.12.5/examples/aio-connect-read.c:96: example_checked: Example 1: "nbd_poll(nbd, -1)" has its value checked in "nbd_poll(nbd, -1) == -1". > libnbd-1.12.5/examples/aio-connect-read.c:128: example_checked: Example 2: "nbd_poll(nbd, -1)" has its value checked in "nbd_poll(nbd, -1) == -1". > libnbd-1.12.5/examples/strict-structured-reads.c:246: example_checked: Example 3: "nbd_poll(nbd, -1)" has its value checked in "nbd_poll(nbd, -1) == -1". > libnbd-1.12.5/ocaml/nbd-c.c:2599: example_assign: Example 4: Assigning: "r" = return value from "nbd_poll(h, timeout)". > libnbd-1.12.5/ocaml/nbd-c.c:2602: example_checked: Example 4 (cont.): "r" has its value checked in "r == -1". > libnbd-1.12.5/python/methods.c:2806: example_assign: Example 5: Assigning: "ret" = return value from "nbd_poll(h, timeout)". > libnbd-1.12.5/python/methods.c:2808: example_checked: Example 5 (cont.): "ret" has its value checked in "ret == -1". > # 178| /* Dispatch work while there are commands in flight. */ > # 179| while (thread->in_flight > 0) > # 180|-> nbd_poll (h, -1); > # 181| } > # 182| > > What it's saying is that in this code base, nbd_poll's return value > was checked by the caller 5 out of 6 times, but ignored here. (This > turned out to be a real bug which we fixed). > > It seems like the check implemented in your patch is: If the return > value is used 0 times anywhere in the code base, change the return > value to 'void'. Coverity would not flag this. > > Maybe a consistent use check is better? Note that the analyzer is currently limited to analyzing a single translation unit at a time, so we would only be able to implement a consistent use check for static functions (this is why the current "return-value-never-used" check only applies to static functions). It may be worthwhile exploring cross-translation unit analysis, although it may be difficult to accomplish while also avoiding reanalyzing the entire tree every time a single translation unit is modified. Alberto
