RFC: sysctl additional functions/macros

[John Baldwin]

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