On Mon, Nov 24, 2025 at 01:50:23PM +0100, Markus Elfring wrote:
> > Finds and converts sized kmalloc-family of allocations into the
> > typed kmalloc_obj-family of allocations.
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc7#n94

"Describe your changes in imperative mood"

I guess I can try rewording this, but it is describing the behavior of
the script. Normally the imperative language for a patch is about what
the patch accomplishes. In this case, I've added a script and it does a
thing itself.

> …
> > +++ b/scripts/coccinelle/api/kmalloc_objs.cocci
> > @@ -0,0 +1,168 @@
> …
> > +// Comments:
> …
> 
> * Please omit such an empty information line.

Agreed; I removed this in v5.

> 
> * Would a field like “Keywords” become helpful?

I only see a few using this -- what benefit is there to adding it? It
seems redundant to the rest of the file, its opening comment, etc.

> > +virtual patch
> 
> Will additional operation modes become relevant after clarification of 
> implementation details?

I am not interested in other modes; if people want to contribute them,
they are welcome. :)

> 
> 
> …
> > +def alloc_array(name):
> > +   func = "FAILED_RENAME"
> > +   if name == "kmalloc_array":
> > +           func = "kmalloc_objs"
> …
> 
> * I suggest to avoid duplicate variable assignments.
> 
> * How do you think about to collaborate with the Python data structure 
> “dictionary”?

I wanted the stderr warning when there was no match, but to return
"FAILED_RENAME" in such a case. Using a dictionary was basically the
same length as the elif stack. If you have an alternative, I'm happy to
replace it with what you come up with.

> 
> 
> …
> > +type TYPE;
> > +TYPE *P;
> > +TYPE INST;
> > +expression VAR;
> > +expression GFP;
> …
> 
> Such repetition of SmPL key words can eventually be also avoided.

That is true, but I like having them separated -- I find it more
readable that way.

-- 
Kees Cook

Reply via email to