From owner-freebsd-arch@FreeBSD.ORG Fri Jan 4 15:43:31 2008 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D51E316A4B3 for ; Fri, 4 Jan 2008 15:43:31 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id 949F013C44B for ; Fri, 4 Jan 2008 15:43:31 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8q) with ESMTP id 227269493-1834499 for multiple; Fri, 04 Jan 2008 10:41:39 -0500 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id m04FhLSk039965; Fri, 4 Jan 2008 10:43:24 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-arch@freebsd.org Date: Fri, 4 Jan 2008 10:42:09 -0500 User-Agent: KMail/1.9.6 References: <477D931D.4000303@elischer.org> In-Reply-To: <477D931D.4000303@elischer.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200801041042.09573.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Fri, 04 Jan 2008 10:43:24 -0500 (EST) X-Virus-Scanned: ClamAV 0.91.2/5363/Fri Jan 4 08:37:27 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Julian Elischer Subject: Re: RFC: sysctl additional functions/macros X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Jan 2008 15:43:31 -0000 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: : 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: : 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: : 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