[v2,0/8] Remove child from struct ctl_table

Message

Joel Granados June 16, 2023, 8:59 a.m. UTC

This is part of the effort to remove the empty element of the ctl_table
structures (used to calculate size) and replace it with an ARRAY_SIZE call. By
replacing the child element in struct ctl_table with a flags element we make
sure that there are no forward recursions on child nodes and therefore set
ourselves up for just using an ARRAY_SIZE. We also added some self tests to
make sure that we do not break anything.

Patchset is separated in 4: parport fixes, selftests fixes, selftests additions and
replacement of child element. Tested everything with sysctl self tests and everything
seems "ok".

1. parport fixes: This is related to my previous series and it plugs a sysct
   table leak in the parport driver. @mcgrof: I'm just leaving this here so we
   don't have to retest the parport stuff

2. Selftests fixes: Remove the prefixed zeros when passing a awk field to the
   awk print command because it was causing $0009 to be interpreted as $0.
   Replaced continue with return in sysctl.sh(test_case) so the test actually
   gets skipped. The skip decision is now in sysctl.sh(skip_test).

3. Selftest additions: New test to confirm that unregister actually removes
   targets. New test to confirm that permanently empty targets are indeed
   created and that no other targets can be created "on top".

4. Replaced the child pointer in struct ctl_table with an enum which is used to
   differentiate between permanently empty targets and non-empty ones.

V2: Replaced the u8 flag with an enumeration.

Comments/feedback greatly appreciated

Best
Joel

Joel Granados (8):
  parport: plug a sysctl register leak
  test_sysctl: Fix test metadata getters
  test_sysctl: Group node sysctl test under one func
  test_sysctl: Add an unregister sysctl test
  test_sysctl: Add an option to prevent test skip
  test_sysclt: Test for registering a mount point
  sysctl: Remove debugging dump_stack
  sysctl: replace child with an enumeration

 drivers/parport/procfs.c                 |  23 ++---
 fs/proc/proc_sysctl.c                    |  82 ++++------------
 include/linux/sysctl.h                   |  14 ++-
 lib/test_sysctl.c                        |  91 ++++++++++++++++--
 tools/testing/selftests/sysctl/sysctl.sh | 115 +++++++++++++++++------
 5 files changed, 214 insertions(+), 111 deletions(-)

Comments

Luis Chamberlain June 18, 2023, 9:33 a.m. UTC | #1

On Fri, Jun 16, 2023 at 10:59:14AM +0200, Joel Granados wrote:
> This is part of the effort to remove the empty element of the ctl_table
> structures (used to calculate size) and replace it with an ARRAY_SIZE call. By
> replacing the child element in struct ctl_table with a flags element we make
> sure that there are no forward recursions on child nodes and therefore set
> ourselves up for just using an ARRAY_SIZE. We also added some self tests to
> make sure that we do not break anything.
> 
> Patchset is separated in 4: parport fixes, selftests fixes, selftests additions and
> replacement of child element. Tested everything with sysctl self tests and everything
> seems "ok".
> 
> 1. parport fixes: This is related to my previous series and it plugs a sysct
>    table leak in the parport driver. @mcgrof: I'm just leaving this here so we
>    don't have to retest the parport stuff
> 
> 2. Selftests fixes: Remove the prefixed zeros when passing a awk field to the
>    awk print command because it was causing $0009 to be interpreted as $0.
>    Replaced continue with return in sysctl.sh(test_case) so the test actually
>    gets skipped. The skip decision is now in sysctl.sh(skip_test).
> 
> 3. Selftest additions: New test to confirm that unregister actually removes
>    targets. New test to confirm that permanently empty targets are indeed
>    created and that no other targets can be created "on top".
> 
> 4. Replaced the child pointer in struct ctl_table with an enum which is used to
>    differentiate between permanently empty targets and non-empty ones.
> 
> V2: Replaced the u8 flag with an enumeration.

Thanks, I dropped the old patches and applied this new set. Pushed out
to sysctl-next.

  Luis

Joel Granados June 19, 2023, 7:32 a.m. UTC | #2

On Sun, Jun 18, 2023 at 02:33:35AM -0700, Luis Chamberlain wrote:
> On Fri, Jun 16, 2023 at 10:59:14AM +0200, Joel Granados wrote:
> > This is part of the effort to remove the empty element of the ctl_table
> > structures (used to calculate size) and replace it with an ARRAY_SIZE call. By
> > replacing the child element in struct ctl_table with a flags element we make
> > sure that there are no forward recursions on child nodes and therefore set
> > ourselves up for just using an ARRAY_SIZE. We also added some self tests to
> > make sure that we do not break anything.
> > 
> > Patchset is separated in 4: parport fixes, selftests fixes, selftests additions and
> > replacement of child element. Tested everything with sysctl self tests and everything
> > seems "ok".
> > 
> > 1. parport fixes: This is related to my previous series and it plugs a sysct
> >    table leak in the parport driver. @mcgrof: I'm just leaving this here so we
> >    don't have to retest the parport stuff
> > 
> > 2. Selftests fixes: Remove the prefixed zeros when passing a awk field to the
> >    awk print command because it was causing $0009 to be interpreted as $0.
> >    Replaced continue with return in sysctl.sh(test_case) so the test actually
> >    gets skipped. The skip decision is now in sysctl.sh(skip_test).
> > 
> > 3. Selftest additions: New test to confirm that unregister actually removes
> >    targets. New test to confirm that permanently empty targets are indeed
> >    created and that no other targets can be created "on top".
> > 
> > 4. Replaced the child pointer in struct ctl_table with an enum which is used to
> >    differentiate between permanently empty targets and non-empty ones.
> > 
> > V2: Replaced the u8 flag with an enumeration.
> 
> Thanks, I dropped the old patches and applied this new set. Pushed out
> to sysctl-next.
Awesome! thx.
> 
>   Luis