Re: [RFC PATCH 00/39] sysctl: replace overloaded extra pointers with typed bounds and context

[Joel Granados]

On Mon, Nov 02, 2026 at 11:40:40PM +0800, Wen Yang wrote:
> 
> 
> On 10/29/26 18:58, David Laight wrote:
> > On Thu, 29 Oct 2026 00:34:39 +0800
> > wen.yang@xxxxxxxxx wrote:
> > 
> > > From: Wen Yang <wen.yang@xxxxxxxxx>
> > > 
> > > struct ctl_table describes an entry with data, maxlen and, until this
> > > series, two untyped misc pointers in extra1 and extra2. Those two pointers
> > > have accumulated at least seven unrelated jobs: a fixed bound, a bound read
> > > through a pointer on every call, an object or namespace pointer, a small
> > > integer cookie encoded into the pointer, a lock signal, NULL standing for
> > > "no bound", and a value the core itself casts to unsigned int while
> > > validating a u8 range. Nothing in the entry says which job applies, so
> > > readers infer it from the handler, and the core gets it wrong where it
> > > treats extra1 as an unsigned int.
> > > 
<... snip ...>
> Hi David, Hi Joel,
> 
> I reworked the context side of the series so that priv is gone entirely;
> and the final tree has no per-entry context pointer.
> A small helper recovers the enclosing object, but it needs the slot
> index from the caller:
> 
>   #define sysctl_table_container(entry, type, member, index) ({       \
>           static_assert(__same_type(*(entry),                         \
>                                     typeof_member(type, member[0])),  \
>                         "pointer type mismatch");                     \
>           (type *)((void *)(entry) - offsetof(type, member) -         \
>                    (size_t)(index) * sizeof(*(entry)));               \
>   })
> 
> The reason plain container_of() is not enough is that the core keeps the
> table as a contiguous array.  For
> 
>   struct addrconf_sysctl_table {
>           struct addrconf_sysctl_ctx context;
>           struct ctl_table table[];
>   };
> 
> the handler receives an interior entry, and
> 
>   container_of(ctl, struct addrconf_sysctl_table, table)
>       == (char *)outer + index * sizeof(struct ctl_table)
> 
> for ctl == &outer->table[index].  It only returns the outer object for
> index 0.  Passing the index explicitly keeps the original registered
> entry usable:
> 
>   static int addrconf_sysctl_forward(const struct ctl_table *ctl, ...)
>   {
>           struct addrconf_sysctl_table *t =
>                   sysctl_table_container(ctl, struct addrconf_sysctl_table,
>                                          table, ADDRCONF_SYSCTL_FORWARDING);
>           const struct addrconf_sysctl_ctx *ctx = &t->context;
>           ...
>   }
> 
> Shared handlers get one wrapper per slot; the common body does not need
> to know its position:
> 
>   #define DEVINET_CONF_WRAPPER(attr)                                  \
>   static int devinet_conf_##attr(const struct ctl_table *ctl, ...)    \
>   {                                                                   \
>           struct devinet_sysctl_table *t =                            \
>                   sysctl_table_container(ctl,                         \
>                           struct devinet_sysctl_table, devinet_vars,  \
>                           IPV4_DEVCONF_##attr - 1);                   \
>           return devinet_conf_proc_common(ctl, &t->context, ...);     \
>   }
> 
> Single-entry tables simply use slot 0:
> 
>   struct devinet_forward_table *t =
>           sysctl_table_container(ctl, struct devinet_forward_table, table,
> 0);
> 
> 
> The series is 20 commits over 134 files, with no priv at any point.  If
> you would prefer a different shape for the helper or the wrappers, I am
> happy to adjust it.
> 
> 
> Thanks,
> Wen
> 
> > David
> > 

Actually: If you want to move the context handling completely outside
sysctl, I would suggest separating into different series and sending
them separately. I'm assuming that the sysctl subsys and the callers are
completely orthogonal after your change

best

Attachment: signature.asc
Description: PGP signature