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

From: Joel Granados
Date: Wed Jul 24 2024 - 14:21:17 EST


On Wed, Jul 24, 2024 at 11:32:26AM +0200, Thomas Weißschuh wrote:
> On 2024-07-24 11:18:17+0000, Joel Granados wrote:
...
> > > > ```
> > >
> > > 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?
>
> Yes, the style fixes were done manually.
Ok. I have created some sed scripts to automate this.

>
> > > Thinking back, there were other "virtual" lines, too.
...
> > int , void *, size_t *, loff_t *);
> > ```
> >
> > >
> > > (It doesn't find anything else, though)
> > Maybe you are missing running it with --include-headers?
>
> There seems to be a slight misunderstanding.
> I meant that it does not find anything *new* on top of the existing
> patch, not that it can fully recreate the patch.
Sorry, misunderstood you.

>
> (I used --include-headers, undid one header hunk from the patch and
> validated that the hunk gets recreated)
ok. That means that "--include-headers" is active.

I actually made further changes on the cocci script for it to catch all
the corner cases that you did by hand. The patch changes substantially
as now there are much less formatting issues introduced by the script.
I'll include you as co-developed-by and in that way give you proper
credit for your work.

In order to reproduce what I did you need to run [1] with this cocci
script [2] and then run [3] with this sed script [4]. I have checked to
see that the same files are changed. The difference between what you and
I did is mainly that coccinelle does not make any formatting decisions.

Will create and send the PR shortly.

Best


[1]
```
make coccicheck MODE=patch SPFLAGS="--in-place --include-headers --smpl-spacing --jobs=8" COCCI=SCRIPT.cocci
```

[2]
```
virtual patch

@r1@
identifier ctl, write, buffer, lenp, ppos;
identifier func !~ "appldata_(timer|interval)_handler|sched_(rt|rr)_handler|rds_tcp_skbuf_handler|proc_sctp_do_(hmac_alg|rto_min|rto_max|udp_port|alpha_beta|auth|probe_interval)";
@@

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 *);

@r4@
identifier func, ctl;
@@

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

@r5@
identifier func, write, buffer, lenp, ppos;
@@

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

[3]
```
sed --in-place -f /tmp/constfy_formating.sed fs/xfs/xfs_sysctl.c kernel/watchdog.c
```

[4]
```
s/^xfs_stats_clear_proc_handler(const struct ctl_table \*ctl,$/xfs_stats_clear_proc_handler(\
\tconst struct ctl_table\t*ctl,/
s/^xfs_panic_mask_proc_handler(const struct ctl_table \*ctl,$/xfs_panic_mask_proc_handler(\
\tconst struct ctl_table\t*ctl,/
s/^xfs_deprecated_dointvec_minmax(const struct ctl_table \*ctl,$/xfs_deprecated_dointvec_minmax(\
\tconst struct ctl_table\t*ctl,/
s/proc_watchdog_common(int which, struct ctl_table \*table/proc_watchdog_common(int which, const struct ctl_table *table/

```

--

Joel Granados