Re: [PATCH 08/11] sysctl: Add size to register_sysctl_init

[Petr Mladek <pmladek@xxxxxxxx>]

On Thu 2023-06-22 16:00:21, Joel Granados wrote:
> On Thu, Jun 22, 2023 at 06:21:48AM +0200, Jiri Slaby wrote:
> > On 21. 06. 23, 15:15, Joel Granados wrote:
> > > On Wed, Jun 21, 2023 at 12:47:58PM +0200, Greg Kroah-Hartman wrote:
> > > > On Wed, Jun 21, 2023 at 11:09:57AM +0200, Joel Granados wrote:
> > > > >   static int __init random_sysctls_init(void)
> > > > >   {
> > > > > -     register_sysctl_init("kernel/random", random_table);
> > > > > +     register_sysctl_init("kernel/random", random_table,
> > > > > +                          ARRAY_SIZE(random_table));
> > > > 
> > > > As mentioned before, why not just do:
> > > > 
> > > > #define register_sysctl_init(string, table)     \
> > > >         __register_sysctl_init(string, table, ARRAY_SIZE(table);
> > > Answered you in the original mail where you suggested it.
> > 
> > I am curious what that was, do you have a link?
> of course. I think you will find it here 
> https://lore.kernel.org/all/20230621123816.ufqbob6qthz4hujx@localhost/

Let me to copy the answer here:

<paste>
I considered this at the outset, but it will not work with callers that
use a pointer instead of the actual array.
Additionally, we would not avoid big commits as we would have to go
looking in all the files where register is called directly or indirectly
and make sure the logic is sound.
</paste>

For the callers using a pointer. A solution would be to create another
wrapper which would take the array size, e.g.

#define register_sysctl_init_limited(string, table, size)       \
        __register_sysctl_init(string, table, size);


And ARRAY_SIZE() is defined in include/linux/kernel.h as:

#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))

It will create a compiler error when either an array[] or *array is
passed.

When using this:

1. The compiler will tell us where the other wrapper is needed.

2. Some locations might need the @size parameter even when a static
   array is passed. For example, neigh_sysctl_register() terminates
   the array early.

   But this will work when __register_sysctl_init() supports
   both ways.I mean that it will stop either on @size or empty
   element, as discussed in the other subthread.

   This should be caught when the final "empty" is removed
   from the particular caller.

Best Regards,
Petr