Date: Fri, 4 Jan 2008 10:42:09 -0500 From: John Baldwin <jhb@freebsd.org> To: freebsd-arch@freebsd.org Cc: Julian Elischer <julian@elischer.org> Subject: Re: RFC: sysctl additional functions/macros Message-ID: <200801041042.09573.jhb@freebsd.org> In-Reply-To: <477D931D.4000303@elischer.org> References: <477D931D.4000303@elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 03 January 2008 08:59:57 pm Julian Elischer wrote:
> I would like to extend the current SYSCTL_INT() with
> SYSCTL_INT_CLAMPED() or similar, where you also supply a
> maximum acceptable value. (and maybe a clue as to what to say if it is
> a bad value).
>
> so many users of SYSCTL_INT don't check for bad values because it's so
> much harder (you need to supply your own handler), and so many
> simple handlers exist fo rthe people that DO check that it seems to
> me that we should provide a pre-canned way to do this....
>
> we are limited to using the existing structure,
> but as we have no existing callers we can redefine
> one element....
In HEAD you can change the ABI of struct sysctl_oid. Or even better, you
could do something like this:
<sys/sysctl.h>:
struct sysctl_int_validator {
int (*siv_validator)(void *arg, int newvalue);
void *siv_arg;
int *siv_data;
}
#define SYSCTL_INT_VALIDATED(parent, nbr, name, access, ptr, validator, arg, descr) \
struct sysctl_int_validator siv__##parent##_##name = { \
(validator), (arg), (ptr) }; \
SYSCTL_OID(parent, nbr, name, CTLTYPE_INT|(access), \
&siv__##parent##_##name, 0, sysctl_handle_validated_int, \
"I", descr)
int sysctl_handle_validated_int(SYSCTL_HANDLER_ARGS);
kern/kern_sysctl.c:
int
sysctl_handle_validated_int(SYSCTL_HANDLER_ARGS)
{
struct sysctl_int_validator *siv;
int error, tmp;
siv = arg1;
if (!siv)
return (EINVAL);
tmp = *siv->siv_data;
error = sysctl_handle_int(oidp, &tmp, 0, req);
if (error || !req->newptr)
return (error);
error = siv->siv_validator(siv->siv_arg, tmp);
if (error == 0)
*siv->siv_data = tmp;
return (error);
}
You can repeat this for the various sysctl types adding a validator handler
for each type. This gives you a framework to add simple validation. You
could then add wrappers to that for specific cases if they are common. For
example, you could implement the _CLAMPED variant like so:
<sys/sysctl.h>:
SYSCTL_INT_CLAMPED(parent, nbr, name, access, ptr, max, descr) \
SYSCTL_INT_VALIDATED(parent, nbr, name, access, ptr, \
sysctl_validate_clamped_int, (void *)max, descr)
int sysctl_validate_clamped_int(void *arg, int newvalue);
kern/kern_sysctl.c:
int
sysctl_validate_clamped_int(void *arg, int newvalue)
{
int max = (intptr_t)arg;
if (newvalue > max)
return (ERANGE);
return (0);
}
You could even add min/max boundaries if desired:
<sys/sysctl.h>:
struct sysctl_int_bounds = {
int sib_min;
int sib_max;
};
SYSCTL_INT_BOUNDED(parent, nbr, name, access, ptr, min, max, descr) \
struct sysctl_int_bounds sib__##parent##_##name = { \
(min), (max) }; \
SYSCTL_INT_VALIDATED(parent, nbr, name, access, ptr, \
sysctl_validate_bounded_int, &sib__##parent##_##name, \
descr)
int sysctl_validate_bounded_int(void *arg, int newvalue);
kern/kern_sysctl.c:
int
sysctl_validate_bounded_int(void *arg, int newwvalue)
{
struct sysctl_bounded_int *sib;
sib = arg;
if (newvalue < sib->sib_min || newvalue > sib->sib_max)
return (ERANGE);
return (0);
}
It would be nice to allow the validator function to be an anonymous routine
like phk suggests. Perhaps something like:
#define SYSCTL_INT_VALIDATOR(parent, nbr, name, access, ptr, descr, validator) \
int \
validate__##parent##_##name(int newvalue) \
validator \
SYSCTL_INT_VALIDATED(parent, nbr, name, access, ptr, \
&validate__##parent##_##name, 0, descr)
would work, though I'm not sure how cpp(1) would really handle that.
You could also inline sysctl_handle_int() into sysctl_handle_validated_int()
if you wanted.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200801041042.09573.jhb>