RFC: sysctl additional functions/macros

[Julian Elischer]

John Baldwin wrote:
> 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.
> 


right, so you've solved that one..  now for global warming..

You might as well go ahead and commit it I think..  :-)