On 2023-12-07 11:43:57+0100, Joel Granados wrote:
> Hey Thomas
>
> You have a couple of test bot issues for your 12/18 patch. Can you
> please address those for your next version.
I have these fixed locally, I assumed Luis would also pick them up
directly until I have a proper v2, properly should have communicated
that.
> On Mon, Dec 04, 2023 at 08:52:13AM +0100, Thomas Weißschuh wrote:
> > Problem description:
> >
> > The kernel contains a lot of struct ctl_table throught the tree.
> > These are very often 'static' definitions.
> > It would be good to make the tables unmodifiable by marking them "const"
> Here I would remove "It would be good to". Just state it: "Make the
> tables unmodifiable...."
Ack.
>
> > to avoid accidental or malicious modifications.
> > This is in line with a general effort to move as much data as possible
> > into .rodata. (See for example[0] and [1])
> If you could find more examples, it would make a better case.
I'll look for some. So far my constifications went in without them :-)
> >
> > Unfortunately the tables can not be made const right now because the
> > core registration functions expect mutable tables.
> >
> > This is for two main reasons:
> >
> > 1) sysctl_{set,clear}_perm_empty_ctl_header in the sysctl core modify
> > the table.
> > 2) The table is passed to the handler function as a non-const pointer.
> >
> > This series migrates the core and all handlers.
> awesome!
>
> >
> > Structure of the series:
> >
> > Patch 1-3: Cleanup patches
> > Patch 4-7: Non-logic preparation patches
> > Patch 8: Preparation patch changing a bit of logic
> > Patch 9-12: Treewide changes to handler function signature
> > Patch 13-14: Adaption of the sysctl core implementation
> > Patch 15: Adaption of the sysctl core interface
> > Patch 16: New entry for checkpatch
> > Patch 17-18: Constification of existing "struct ctl_table"s
> >
> > Tested by booting and with the sysctl selftests on x86.
> >
> > Note:
> >
> > This is intentionally sent only to a small number of people as I'd like
> > to get some more sysctl core-maintainer feedback before sending this to
> > essentially everybody.
> When you do send it to the broader audience, you should chunk up your big
> patches (12/18 and 11/18) and this is why:
> 1. To avoid mail rejections from lists:
> You have to tell a lot of people about the changes in one mail. That
> will make mail header too big for some lists and it will be rejected.
> This happened to me with [3]
> 2. Avoid being rejected for the wrong reasons :)
> Maintainers are busy ppl and sending them a set with so many files
> may elicit a rejection on the grounds that it involves too many
> subsystems at the same time.
> I suggest you chunk it up with directories in mind. Something similar to
> what I did for [4] where I divided stuff that when for fs/*, kernel/*,
> net/*, arch/* and drivers/*. That will complicate your patch a tad
> because you have to ensure that the tree can be compiled/run for every
> commit. But it will pay off once you push it to the broader public.
This will break bisections. All function signatures need to be switched
in one step. I would strongly like to avoid introducing broken commits.
The fact that these big commits have no functional changes at all makes
me hope it can work.
