2023-11-28 14:08:13

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 07/10] printk: Remove the now superfluous sentinel elements from ctl_table array

On Tue 2023-11-07 14:45:07, Joel Granados via B4 Relay wrote:
> From: Joel Granados <[email protected]>
>
> This commit comes at the tail end of a greater effort to remove the
> empty elements at the end of the ctl_table arrays (sentinels) which
> will reduce the overall build time size of the kernel and run time
> memory bloat by ~64 bytes per sentinel (further information Link :
> https://lore.kernel.org/all/ZO5Yx5JFogGi%[email protected]/)
>
> rm sentinel element from printk_sysctls
>
> Signed-off-by: Joel Granados <[email protected]>

I am a bit sceptical if the size and time reduction is worth the
effort. I feel that this change makes the access a bit less secure.

Well, almost all arrays are static so that it should just work.
The patch does what it says. Feel free to use:

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr


2023-12-04 08:56:50

by Joel Granados

[permalink] [raw]
Subject: Re: [PATCH 07/10] printk: Remove the now superfluous sentinel elements from ctl_table array

Hey Petr

I missed this message somehow....

On Tue, Nov 28, 2023 at 03:07:43PM +0100, Petr Mladek wrote:
> On Tue 2023-11-07 14:45:07, Joel Granados via B4 Relay wrote:
> > From: Joel Granados <[email protected]>
> >
> > This commit comes at the tail end of a greater effort to remove the
> > empty elements at the end of the ctl_table arrays (sentinels) which
> > will reduce the overall build time size of the kernel and run time
> > memory bloat by ~64 bytes per sentinel (further information Link :
> > https://lore.kernel.org/all/ZO5Yx5JFogGi%[email protected]/)
> >
> > rm sentinel element from printk_sysctls
> >
> > Signed-off-by: Joel Granados <[email protected]>
>
> I am a bit sceptical if the size and time reduction is worth the
> effort. I feel that this change makes the access a bit less secure.
In what way "less secure"? Can you expand on that?

Notice that if you pass a pointer to the register functions, you will
get a warning/error on compilation.

>
> Well, almost all arrays are static so that it should just work.
> The patch does what it says. Feel free to use:
Thx for the review. will do.

>
> Reviewed-by: Petr Mladek <[email protected]>
>
> Best Regards,
> Petr

--

Joel Granados


Attachments:
(No filename) (1.25 kB)
signature.asc (673.00 B)
Download all attachments

2023-12-06 09:56:22

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 07/10] printk: Remove the now superfluous sentinel elements from ctl_table array

On Mon 2023-12-04 09:56:28, Joel Granados wrote:
> Hey Petr
>
> I missed this message somehow....
>
> On Tue, Nov 28, 2023 at 03:07:43PM +0100, Petr Mladek wrote:
> > On Tue 2023-11-07 14:45:07, Joel Granados via B4 Relay wrote:
> > > From: Joel Granados <[email protected]>
> > >
> > > This commit comes at the tail end of a greater effort to remove the
> > > empty elements at the end of the ctl_table arrays (sentinels) which
> > > will reduce the overall build time size of the kernel and run time
> > > memory bloat by ~64 bytes per sentinel (further information Link :
> > > https://lore.kernel.org/all/ZO5Yx5JFogGi%[email protected]/)
> > >
> > > rm sentinel element from printk_sysctls
> > >
> > > Signed-off-by: Joel Granados <[email protected]>
> >
> > I am a bit sceptical if the size and time reduction is worth the
> > effort. I feel that this change makes the access a bit less secure.
> In what way "less secure"? Can you expand on that?
>
> Notice that if you pass a pointer to the register functions, you will
> get a warning/error on compilation.

I have vague memories that some arrays were not static or the length
has been somehow manipulated. But I might be wrong.

You are right that it should be safe with the static arrays.
And the NULL sentinel might be more error-prone after all.

Let's forget my mumbles.

Best Regards,
Petr