git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 0/6] add and apply a rule to find "unused" init+free

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 11, 2022, 10:54 UTC
Message-ID
<220711.86wncj7w8p.gmgdl@evledraar.gmail.com>
In-Reply-To
<YsvwLIfjZffUWOP/@coredump.intra.peff.net>
On Mon, Jul 11 2022, Jeff King wrote:
Show 9 quoted lines
> On Tue, Jul 05, 2022 at 03:46:54PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> This series adds a coccinelle rule to find and remove code where the
>> only reference to a variable in a given function is to malloc() &
>> free() it, where "malloc" and "free" also match
>> "strbuf_init/strbuf_release", and then later in the series anything
>> that looks like a init/free pattern.
>
> As before, I'm perfectly happy with the actual transformations here.
Thanks!
Show 13 quoted lines
>> Changes since v3[1]
>> 
>>  * Add a "coccicheck-test" target in an early and new patch, the
>>    structure mirrors that of coccinelle.git's own tests. As the
>>    diffstat shows we have a *.c and *.res file which is C code
>>    before/after a *.cocci rule is applied.
>
> I have mixed feelings on this. I'm not opposed per se, and I even like
> the fact that it provides examples of what we expect a rule to do. But I
> worry about a world where the cost of using coccinelle goes up because
> now transformations need to have tests along with them. The idea of
> cocci is to make your life a little simpler and more efficient when
> fixing up the code base (as opposed to perl, etc).

That's fair, and I suppose we could add some really complex tests that e.g. include the original headers.

But right now these just work on stand-alone *.c files with no includes, and the runtime is thus trivial. So I thought it was OK to run it along with the main job.

Show 12 quoted lines
> But if it comes with a bunch of boilerplate tasks, that might no
> longer be the case. I'm already often on the fence about using cocci
> just because of the hours I've sunk into debugging and puzzling things
> out.
>
> I dunno. Maybe the bar is higher for stuff like this that we expect to
> continue to find problems as time goes on (because it is not "here is a
> transition", but "this is a common and easy mistake to make").
>
> So again, I'm not really opposed to this patch in particular, but I want
> to express caution at the direction we might be going, and at applying
> new rules over-zealously.

Yeah, we definitely need to be more careful with "ongoing" rules like these, but hopefully between the tests & lack of false positives this one looks sane...

Show 6 quoted lines
>>  * We now catch init/reset patterns as well as init/release fully
>>    (i.e. for the "struct strbuf" early on)
>
> Yeah, that makes sense.
>
> -Peff
Previous: Jeff King
Message 24 of 24 in “cocci: add and apply a rule to find "unused" variables”
  1. cocci: add and apply a rule to find "unused" variablesÆvar Arnfjörð Bjarmason, May 20, 2022
  2. 0/2 add and apply a rule to find "unused" init+freeÆvar Arnfjörð Bjarmason, Jun 21, 2022
  3. 1/2 cocci: add and apply a rule to find "unused" variablesÆvar Arnfjörð Bjarmason, Jun 21, 2022
  4. Junio C HamanoJun 22, 2022
  5. 2/2 cocci: remove "when strict" from unused.cocciÆvar Arnfjörð Bjarmason, Jun 21, 2022
  6. 0/4 add and apply a rule to find "unused" init+freeÆvar Arnfjörð Bjarmason, Jul 1, 2022
  7. 1/4 cocci: add and apply a rule to find "unused" strbufsÆvar Arnfjörð Bjarmason, Jul 1, 2022
  8. Jeff KingJul 1, 2022
  9. Eric SunshineJul 1, 2022
  10. 2/4 cocci: catch unused "strbuf" using an xmalloc() patternÆvar Arnfjörð Bjarmason, Jul 1, 2022
  11. 4/4 cocci: generalize "unused" rule to cover more than "strbuf"Ævar Arnfjörð Bjarmason, Jul 1, 2022
  12. 3/4 cocci: remove "when strict" from unused.cocciÆvar Arnfjörð Bjarmason, Jul 1, 2022
  13. Eric SunshineJul 1, 2022
  14. Jeff KingJul 1, 2022
  15. 0/6 add and apply a rule to find "unused" init+freeÆvar Arnfjörð Bjarmason, Jul 5, 2022
  16. 2/6 Makefile & .gitignore: ignore & clean "git.res", not "*.res"Ævar Arnfjörð Bjarmason, Jul 5, 2022
  17. 1/6 Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGSÆvar Arnfjörð Bjarmason, Jul 5, 2022
  18. 3/6 cocci: add a "coccicheck-test" target and test *.cocci rulesÆvar Arnfjörð Bjarmason, Jul 5, 2022
  19. 4/6 cocci: have "coccicheck{,-pending}" depend on "coccicheck-test"Ævar Arnfjörð Bjarmason, Jul 5, 2022
  20. 5/6 cocci: add and apply a rule to find "unused" strbufsÆvar Arnfjörð Bjarmason, Jul 5, 2022
  21. 6/6 cocci: generalize "unused" rule to cover more than "strbuf"Ævar Arnfjörð Bjarmason, Jul 5, 2022
  22. Junio C HamanoJul 6, 2022
  23. Jeff KingJul 11, 2022
  24. Ævar Arnfjörð BjarmasonJul 11, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.