Re: [PATCH i-g-t 7/7] scripts/cocci: Add iterators.cocci

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Dec 04, 2025 at 01:56:50PM +0100, Hajda, Andrzej wrote:
> W dniu 24.11.2025 o 23:40, Ville Syrjälä pisze:
> > On Mon, Nov 24, 2025 at 11:49:40PM +0200, Ville Syrjälä wrote:
> >> On Thu, Nov 20, 2025 at 09:14:15PM +0200, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Coccinelle often chokes on magic iterator macros. We
> >>> can lend it a hand by explicitly declaring those macros
> >>> as iterators. Start collecting them in
> >>> scripts/iterators.cocci which people can easily include
> >>> in their cocci scripts.
> >>>
> >>> The "struct dummy" is there because coccinelle doesn't
> >>> like an empty script. Dunno if there's a cleaner way
> >>> around that?
> >>>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>   scripts/iterators.cocci | 53 +++++++++++++++++++++++++++++++++++++++++
> >>>   1 file changed, 53 insertions(+)
> >>>   create mode 100644 scripts/iterators.cocci
> >>>
> >>> diff --git a/scripts/iterators.cocci b/scripts/iterators.cocci
> >>> new file mode 100644
> >>> index 000000000000..90d3ae437a15
> >>> --- /dev/null
> >>> +++ b/scripts/iterators.cocci
> >>> @@ -0,0 +1,53 @@
> >>> +// SPDX-License-Identifier: MIT
> >>> +//
> >>> +// Use with
> >>> +// #include "scripts/iterators.cocci"
> >>> +// at the start of your cocci script
> >>> +
> >>> +@@
> >>> +iterator name for_each_collection_data;
> >>> +iterator name for_each_combination;
> >>> +iterator name for_each_connected_output;
> >>> +iterator name for_each_connector_mode;
> >>> +iterator name for_each_ctx_engine;
> >>> +iterator name for_each_disconnected_output;
> >>> +iterator name for_each_format;
> >>> +iterator name for_each_if;
> >>> +iterator name for_each_output;
> >>> +iterator name for_each_pipe;
> >>> +iterator name for_each_pipe_static;
> >>> +iterator name for_each_pipe_with_single_output;
> >>> +iterator name for_each_pipe_with_valid_output;
> >>> +iterator name for_each_plane_on_pipe;
> >>> +iterator name for_each_prime_number;
> >>> +iterator name for_each_sriov_enabled_vf;
> >>> +iterator name for_each_sriov_num_vfsfor_each_sriov_vf;
> >>> +iterator name for_each_sriov_num_vfs_in_rangefor_each_sriov_vf_in_range;
> >>> +iterator name for_each_sriov_vf;
> >>> +iterator name for_each_sriov_vf_in_range;
> >>> +iterator name for_each_subset;
> >>> +iterator name for_each_subset;
> >>> +iterator name for_each_sysfs_gt_dirfd;
> >>> +iterator name for_each_sysfs_gt_path;
> >>> +iterator name for_each_sysfs_tile_dirfd;
> >>> +iterator name for_each_tiling;
> >>> +iterator name for_each_valid_output_on_pipe;
> >>> +iterator name for_each_variation_nr;
> >>> +iterator name for_each_variation_r;
> > 
> >>> +iterator name igt_for_each_drm_client;
> >>> +iterator name igt_list_for_each_entry;
> >>> +iterator name igt_list_for_each_entry_reverse;
> >>> +iterator name igt_list_for_each_entry_safe;
> >>> +iterator name igt_list_for_each_entry_safe_reverse;
> > 
> > And now that I think about it, I seem to recall cocci having
> > some kind of builtin heuristic for this...
> > 
> > Indeed, parsing_c/parsing_hacks.ml says:
> > let regexp_foreach = Str.regexp_case_fold
> >    ".*\\(for_?each\\|for_?all\\|iterate\\|loop\\|walk\\|scan\\|each\\|for\\)"
> > 
> > So I guess we don't actually need to declare any of the
> > iterators above. IIRC I generated that list with a simple
> > 'grep for_each', more or less just in case.
> > 
> >>> +iterator name igt_dynamic;
> >>> +iterator name igt_dynamic_f;
> >>> +iterator name igt_fixture;
> >>> +iterator name igt_fork;
> >>> +iterator name igt_require;
> >>> +iterator name igt_subtest;
> >>> +iterator name igt_subtest_f;
> >>> +iterator name igt_subtest_group;
> >>> +iterator name igt_subtest_with_dynamic;
> >>> +iterator name igt_subtest_with_dynamic_f;
> >>
> >> Couple of other needed iterators I apparently forgot to include here:
> >>
> >> +iterator name igt_until_timeout;
> >> +iterator name igt_while_interruptible;
> > 
> > All of this stuff I had to add manually to actually make
> > cocci parse the code succesfully. And we can see none of it
> > will match that heuristic regexp.
> > 
> >>
> >> I should also check if cocci supports regexp with iterator names since
> >> that could reduce this list quite a bit...
> > 
> > Doesn't seem to be the case. But at least I can reduce
> > the list due to the builtin heuristic.
> 
> Nice work. What cocci coverage in percents do you achieve with this?

Original was:
 NB total files = 1000; perfect = 479; pbs = 521; timeout = 0; =========> 47%
 nb good = 2725082,  nb passed = 6531 =========> 0.23% passed
 nb good = 2725082,  nb bad = 61818 =========> 97.79% good or passed

This is what I get after this stuff:
 NB total files = 1000; perfect = 522; pbs = 478; timeout = 0; =========> 52%
 nb good = 2728902,  nb passed = 6721 =========> 0.24% passed
 nb good = 2728902,  nb bad = 57989 =========> 97.92% good or passed

That's all pretty much due to the igt_main stuff because I couldn't
find any way to use the iterators.cocci script with --parse-c.

Using the macro based approach for the iterators I get:
 NB total files = 1000; perfect = 892; pbs = 108; timeout = 0; =========> 89%
 nb good = 2776744,  nb passed = 5443 =========> 0.19% passed
 nb good = 2776744,  nb bad = 10147 =========> 99.64% good or passed

So I guess that's about what one may now expect when using
iterators.cocci.

Anyways, this is all pushed now. Thanks for the acks.


PS.
For including the cocci scripts in the commit message I switched to
using the following format:
 #include "scripts/iterators.cocci"

blah
blah

Ie. just put one extra space before the '#' so that git doesn't
think it's a comment and strip it.

There is a way to change the comment character in git, but I'm not
sure it's a good idea if only some people do that. Someone else
grabbing those commits and doing an interactive rebase might get
bitten by it.

Cocci doesn't even seem to mind the extra space, so you can still
copy-paste the script from the commit message as is.

> 
> I have asked once cocci author about the same issue some time ago, she 
> proposed different solution [1] - to provide --macro-file-builtins with
> defines for problematic macros.
> Not sure which approach better solves IGT issues, defines seems for me 
> more flexible, but I do not feel an expert in this area.
> 
> Anyway adding parenthesis to igt macros makes them look more 'c like' :)
> 
> Acked-by: Andrzej Hajda <andrzej.hajda@intel.com>
> 
> [1]: 
> https://lore.kernel.org/cocci/4aed4763-9b3c-c21f-693a-e7d4281f8384@inria.fr/
> 
> Regards
> Andrzej
> 
> > 
> >>
> >>> +@@
> >>> +struct dummy
> >>> -- 
> >>> 2.49.1
> >>
> >> -- 
> >> Ville Syrjälä
> >> Intel
> > 

-- 
Ville Syrjälä
Intel