Re: [PATCH 00/11] Remove the end element in sysctl table arrays.

From: Greg KH <greg@kroah.com>
To: Joel Granados <j.granados@samsung.com>
Cc: mcgrof@kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH 00/11] Remove the end element in sysctl table arrays.
Date: Wed, 21 Jun 2023 12:46:47 +0200	[thread overview]
Message-ID: <2023062117-federal-dash-cf50@gregkh> (raw)
In-Reply-To: <20230621091000.424843-1-j.granados@samsung.com>

On Wed, Jun 21, 2023 at 11:09:49AM +0200, Joel Granados wrote:
> This is part of the effort to remove the empty element from the ctl_table
> structures (used to calculate size) and replace it with the ARRAY_SIZE macro.
> The "sysctl: Remove the end element in sysctl table arrays" commit is the one that
> actually removes the empty element. With a "yesall" configuration the bloat-o-meter
> says that 9158 bytes where saved (report at the end of the cover letter).

9k in ram or read-only memory?

> Main changes:
> 1. Add the ctl_table size into the ctl_table_header
> 2. Remove the empty element at the end of all ctl_table arrays
> 
> Commit Overview:
> 1. There are preparation commits that make sure that we have the
>    ctl_table_header in all the places that we need to have the array size.
>       sysctl: Prefer ctl_table_header in proc_sysct
>       sysctl: Use the ctl header in list ctl_table macro
>       sysctl: Add ctl_table_size to ctl_table_header
> 
> 2. Add size to relevant register calls. Calculate the ctl_table array size
>    where register_sysctl is called. Add a table_size argument to the relevant
>    sysctl register functions (init_header, __register_sysctl_table,
>    register_net_sysctl, register_sysctl and register_sysctl_init). Important to
>    note that these commits do NOT change the way we calculate size; they plumb
>    things in preparation for the empty element removal commit. Care is taken to
>    leave the tree in a state where it can be compiled which is the reason to
>    not separate the "big" commits (like "sysctl: Add size to the
>    register_net_sysctl function"). If you have an alternative way of dealing
>    with such a big commit while leaving it in a compilable state, please let me
>    know.
>       sysctl: Add size argument to init_header
>       sysctl: Add a size arg to __register_sysctl_table
>       sysctl: Add size to the register_net_sysctl function
>       sysctl: Add size to register_sysctl
>       sysctl: Add size to register_sysctl_init

Why not make these calls automatically calculate the size based on the
structure passed into them by using a #define instead of having to touch
the code everywhere?  That would make this much simpler AND make it
impossible for future people to get this wrong.

thanks,

greg k-h