Re: [PATCH 2/4] aio: Remove the now superfluous sentinel elements from ctl_table array

On Tue, Nov 07, 2023 at 07:42:31PM -0800, Eric Biggers wrote:
> On Tue, Nov 07, 2023 at 02:44:21PM +0100, Joel Granados via B4 Relay wrote:
> > [PATCH 2/4] aio: Remove the now superfluous sentinel elements from ctl_table array
> 
> The commit prefix should be "fs:".
Will do

> 
> > Remove sentinel elements ctl_table struct. Special attention was placed in
> > making sure that an empty directory for fs/verity was created when
> > CONFIG_FS_VERITY_BUILTIN_SIGNATURES is not defined. In this case we use the
> > register sysctl call that expects a size.
> [...]
> > diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> > index d071a6e32581..8191bf7ad706 100644
> > --- a/fs/verity/fsverity_private.h
> > +++ b/fs/verity/fsverity_private.h
> > @@ -122,8 +122,8 @@ void __init fsverity_init_info_cache(void);
> >  
> >  /* signature.c */
> >  
> > -#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> >  extern int fsverity_require_signatures;
> > +#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> >  int fsverity_verify_signature(const struct fsverity_info *vi,
> >  			      const u8 *signature, size_t sig_size);
> >  
> > diff --git a/fs/verity/init.c b/fs/verity/init.c
> > index a29f062f6047..e31045dd4f6c 100644
> > --- a/fs/verity/init.c
> > +++ b/fs/verity/init.c
> > @@ -13,7 +13,6 @@
> >  static struct ctl_table_header *fsverity_sysctl_header;
> >  
> >  static struct ctl_table fsverity_sysctl_table[] = {
> > -#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> >  	{
> >  		.procname       = "require_signatures",
> >  		.data           = &fsverity_require_signatures,
> > @@ -23,14 +22,17 @@ static struct ctl_table fsverity_sysctl_table[] = {
> >  		.extra1         = SYSCTL_ZERO,
> >  		.extra2         = SYSCTL_ONE,
> >  	},
> > -#endif
> > -	{ }
> >  };
> >  
> >  static void __init fsverity_init_sysctl(void)
> >  {
> > +#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> >  	fsverity_sysctl_header = register_sysctl("fs/verity",
> >  						 fsverity_sysctl_table);
> > +#else
> > +	fsverity_sysctl_header = register_sysctl_sz("fs/verity",
> > +						 fsverity_sysctl_table, 0);
> > +#endif
> >  	if (!fsverity_sysctl_header)
> >  		panic("fsverity sysctl registration failed");
> 
> This does not make sense, and it causes a build error when CONFIG_FS_VERITY=y
> and CONFIG_FS_VERITY_BUILTIN_SIGNATURES=n.
> 
> I think all you need to do is delete the sentinel element, the same as
> everywhere else.  I just tested it, and it works fine.
Indeed. good catch and thx for testing. I'll adjust it on my V2.

> 
> BTW, the comments for register_sysctl_sz() and __register_sysctl_table() are
> outdated, as they still say "A completely 0 filled entry terminates the table."
For now this is still "technically" correct. However, this will be
removed on the last patchset when we actually delete the check for the
sentinel element.
For now I'll leave it like it is, but I'll double check to make sure
that I remove it at the end.

Best
> 
> - Eric

-- 

Joel Granados

-----BEGIN PGP SIGNATURE-----

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmVLRjsACgkQupfNUreW
QU/+QQv+JAZ2egwDryj1fBPctk+jcFkv5AFT95mMW0Hy10yYejq2clAC6/4VpFPL
Zw7EKmOW7gx/ZH4lLvjKbK0w9MZyUigYGBml+WQjyHoVFnm9Lij3PZ2nUEbZHMbC
xYYt3TfWlMd9EKSsZu248QyT1QYUqdVsPadtMciHeLOHJUKwbq16BUgkIQ2uL7yw
+rNN7XjYxwwdHhOdOW9+CaqzKkTTxYm3jwCUB1g/Lb01yhky+pgOlnKSGRj11WSo
mVOSdQmMiN90KkTYfqF8pXi7snELIRo6Bjfp2v3pa6ea6DFkj4IA8Eh0SMu9eDOL
yffPi1tzs7GHx87Rz4Xw3HL+PGsKwE5jVh+WI/NOR3ECMieTEHyeBP0b6CctG1Qz
Y1cX1rzSae8Y+NfQ/uML6S+L6d++SbVUuNFim3hAslWjQzSfp6OxsD/3YJpQvWNU
H6yInkAYVBv9kXJdpylR0tiRthbrKul+NEjL1/FvJVi2460ZC3QzEThsT/wNhLJ2
cXMro6Zk
=sgbZ
-----END PGP SIGNATURE-----