On Fri, Jul 29, 2022 at 4:33 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > Fix a bug in 0139c58ab95 (revisions API users: add "goto cleanup" for > release_revisions(), 2022-04-13), in that commit a release_revisions() > call was added to this function, but it never did anything due to this > TODO memset() added in fe1a21d5267 (fast-rebase: demonstrate > merge-ort's API via new test-tool command, 2020-10-29). > > Simply removing the memset() will fix the "cmdline" which can be seen > when running t5520-pull.sh. > > This sort of thing could be detected automatically with a rule similar > to the unused.cocci merged in 7fa60d2a5b6 (Merge branch > 'ab/cocci-unused' into next, 2022-07-11). The following rule on top > would catch the case being fixed here: > > @@ > type T; > identifier I; > identifier REL1 =~ "^[a-z_]*_(release|reset|clear|free)$"; > identifier REL2 =~ "^(release|clear|free)_[a-z_]*$"; > @@ > > - memset(\( I \| &I \), 0, ...); > ... when != \( I \| &I \) > ( > \( REL1 \| REL2 \)( \( I \| &I \), ...); > | > \( REL1 \| REL2 \)( \( &I \| I \) ); > ) > ... when != \( I \| &I \) > > That rule should arguably use only &I, not I (as we might be passed a > pointer). He distinction would matter if anyone cared about the s/He/The/ > side-effects of a memset() followed by release() of a pointer to a > variable passed into the function. > > As such a pattern would be at best very confusing, and most likely > point to buggy code as in this case, the above rule is probably fine > as-is. > > But as this rule only found one such bug in the entire codebase let's > not add it to contrib/coccinelle/unused.cocci for now, we can always > dig it up in the future if it's deemed useful. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
