Re: [PATCH v2] sysctl: treewide: constify the ctl_table argument of proc_handlers

[Joel Granados]

On Wed, Jul 24, 2024 at 10:56:10AM +0200, Thomas Weißschuh wrote:
> On 2024-07-24 10:41:24+0000, Joel Granados wrote:
...
> > > > to be in master before we send the PR to Linus. Will check these three
> > > > dependencies on Wednesday next week and send your V2 [3] if I see that
> > > > it applies cleanly.
> > > 
> > > All dependency PRs (sysctl, net, mm) are now merged.
> > > My compilation tests all succeed now.
> > 
> > How did you apply the coccinelle script? I ask because if I do this:
> > ```
> >  make coccicheck MODE=patch SPFLAGS="--in-place --include-headers" COCCI=SCRIPT
> > ```
> > 
> > I have to add "virtual patch" to the first line of the script you had
> > sent. So it would look like this:
> > ```
> > virtual patch
> > 
> > @@
> > identifier func, ctl, write, buffer, lenp, ppos;
> > @@
> > 
> > int func(
> > - struct ctl_table *ctl,
> > + const struct ctl_table *ctl,
> >   int write, void *buffer, size_t *lenp, loff_t *ppos)
> > { ... }
> > ```
> 
> Yes, IIRC I got told during one review to drop it.
> But for me it is also necessary.
I also remember a comment from the XFS part of the patches where you
changed the formatting on some functions. What did you do to address
this? Did you change them manually? Or do you have a script?

> 
> Thinking back, there were other "virtual" lines, too.
> Maybe those were to ones that needed to be removed, but
> "virtual patch" should stay.
Understood.

> 
> > Additionally, this cocci script is not changing the header files. Even
> > if I pass --include-headers. Did you change those by hand?
> 
> Yes, I changed these manually, originally.
> 
> To do it through the script, you need a second subpatch:
> 
> ```
> @@
> identifier func, ctl, write, buffer, lenp, ppos;
> @@
> 
> int func(
> - struct ctl_table *ctl,
> + const struct ctl_table *ctl,
>   int write, void *buffer, size_t *lenp, loff_t *ppos);
> ```
Yes. But you are still missing one more subpatch to catch the function
declarations in header files that don't name the arguments; like the
ones in include/linux/mm.h. This is what I used for those:
```
@r3@
identifier func;
@@

int func(
- struct ctl_table *,
+ const struct ctl_table *,
  int , void *, size_t *, loff_t *);
```

> 
> (It doesn't find anything else, though)
Maybe you are missing running it with --include-headers?

This is the full cocci script that I have:
```
virtual patch

@r1@
identifier func, ctl, write, buffer, lenp, ppos;
@@

int func(
- struct ctl_table *ctl,
+ const struct ctl_table *ctl,
  int write, void *buffer, size_t *lenp, loff_t *ppos);

@r2@
identifier func, ctl, write, buffer, lenp, ppos;
@@

int func(
- struct ctl_table *ctl,
+ const struct ctl_table *ctl,
  int write, void *buffer, size_t *lenp, loff_t *ppos)
{ ... }

@r3@
identifier func;
@@

int func(
- struct ctl_table *,
+ const struct ctl_table *,
  int , void *, size_t *, loff_t *);
```




-- 

Joel Granados