On Wed, Apr 28, 2021 at 12:06:26PM +0200, Alexander Bluhm wrote:
> On Tue, Apr 27, 2021 at 08:22:20PM -0700, Greg Steuck wrote:
> > Vitaliy Makkoveev <m...@openbsd.org> writes:
> >
> > > On Tue, Apr 27, 2021 at 09:12:56PM +0200, Alexander Bluhm wrote:
> > >> On Tue, Apr 27, 2021 at 12:18:02PM -0600, Theo de Raadt wrote:
> > >> > Actually, your variation seems pretty good. Is there any reason to not
> > >> > use this type of define?
> > >>
> > >> This would look like this.
> > >>
> > >> I think sysctl_int() and sysctl_rdint() should be the primitive
> > >> functions. This brings us back the 4.4BSD implementation. Then
> > >> sysctl_int_bounded() builds the magic on top like sysctl_int_lower()
> > >> does it. sysctl_bounded_arr() is a wrapper around it.
> > >>
> > >> I just added a few defines that my simple grep found. We could
> > >> search for more.
> > >
> > > This looks much better, I like the direction.
> >
> > I like this too. I somehow got the impression that macros are severely
> > frowned upon and didn't offer this kind of interface before.
> >
> > If you get this submitted, I can do a pass through the codebase to be
> > sure we catch them all.
>
> Anyone who wants to OK it?
>
> bluhm
Could you convert all related structures? At least `cpuctl_vars' for
i386 and amd64 cases, `kern_vars', `hw_vars', `fusefs_vars', and
`ffs_vars' are missing in your diff.
>
> Index: kern/kern_sysctl.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.390
> diff -u -p -r1.390 kern_sysctl.c
> --- kern/kern_sysctl.c 23 Apr 2021 07:21:02 -0000 1.390
> +++ kern/kern_sysctl.c 28 Apr 2021 10:04:02 -0000
> @@ -818,7 +818,8 @@ debug_sysctl(int *name, u_int namelen, v
> * Reads, or writes that lower the value
> */
> int
> -sysctl_int_lower(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int
> *valp)
> +sysctl_int_lower(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
> + int *valp)
> {
> unsigned int oval = *valp, val = *valp;
> int error;
> @@ -841,35 +842,40 @@ sysctl_int_lower(void *oldp, size_t *old
> int
> sysctl_int(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int *valp)
> {
> - return (sysctl_int_bounded(oldp, oldlenp, newp, newlen, valp, 0, 0));
> -}
> -
> -int
> -sysctl_int_bounded(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
> - int *valp, int minimum, int maximum)
> -{
> int error = 0;
> - int val;
>
> if (oldp && *oldlenp < sizeof(int))
> return (ENOMEM);
> if (newp && newlen != sizeof(int))
> return (EINVAL);
> *oldlenp = sizeof(int);
> - val = *valp;
> if (oldp)
> - error = copyout(&val, oldp, sizeof(int));
> + error = copyout(valp, oldp, sizeof(int));
> if (error == 0 && newp)
> - error = copyin(newp, &val, sizeof(int));
> - if (error)
> - return (error);
> - if (minimum == maximum || (minimum <= val && val <= maximum))
> - *valp = val;
> - else
> - error = EINVAL;
> + error = copyin(newp, valp, sizeof(int));
> return (error);
> }
>
> +int
> +sysctl_int_bounded(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
> + int *valp, int minimum, int maximum)
> +{
> + int val = *valp;
> + int error;
> +
> + /* read only */
> + if (newp == NULL || minimum > maximum)
> + return (sysctl_rdint(oldp, oldlenp, newp, *valp));
> +
> + if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
> + return (error);
> + /* bounded and not within limits */
> + if (minimum < maximum && (val < minimum || maximum < val))
> + return (EINVAL);
> + *valp = val;
> + return (0);
> +}
> +
> /*
> * As above, but read-only.
> */
> @@ -901,14 +907,8 @@ sysctl_bounded_arr(const struct sysctl_b
> return (ENOTDIR);
> for (i = 0; i < valplen; ++i) {
> if (valpp[i].mib == name[0]) {
> - if (valpp[i].minimum <= valpp[i].maximum) {
> - return (sysctl_int_bounded(oldp, oldlenp, newp,
> - newlen, valpp[i].var, valpp[i].minimum,
> - valpp[i].maximum));
> - } else {
> - return (sysctl_rdint(oldp, oldlenp, newp,
> - *valpp[i].var));
> - }
> + return (sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> + valpp[i].var, valpp[i].minimum, valpp[i].maximum));
> }
> }
> return (EOPNOTSUPP);
> Index: kern/kern_tc.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_tc.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 kern_tc.c
> --- kern/kern_tc.c 23 Feb 2021 04:44:31 -0000 1.71
> +++ kern/kern_tc.c 28 Apr 2021 10:04:02 -0000
> @@ -829,7 +829,7 @@ inittimecounter(void)
> }
>
> const struct sysctl_bounded_args tc_vars[] = {
> - { KERN_TIMECOUNTER_TICK, &tc_tick, 1, 0 },
> + { KERN_TIMECOUNTER_TICK, &tc_tick, SYSCTL_INT_READONLY },
> { KERN_TIMECOUNTER_TIMESTEPWARNINGS, ×tepwarnings, 0, 1 },
> };
>
> Index: kern/sysv_sem.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/sysv_sem.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 sysv_sem.c
> --- kern/sysv_sem.c 17 Nov 2020 03:23:54 -0000 1.60
> +++ kern/sysv_sem.c 28 Apr 2021 10:04:02 -0000
> @@ -860,10 +860,10 @@ sema_reallocate(int val)
> }
>
> const struct sysctl_bounded_args sysvsem_vars[] = {
> - { KERN_SEMINFO_SEMUME, &seminfo.semume, 1, 0 },
> - { KERN_SEMINFO_SEMUSZ, &seminfo.semusz, 1, 0 },
> - { KERN_SEMINFO_SEMVMX, &seminfo.semvmx, 1, 0 },
> - { KERN_SEMINFO_SEMAEM, &seminfo.semaem, 1, 0 },
> + { KERN_SEMINFO_SEMUME, &seminfo.semume, SYSCTL_INT_READONLY },
> + { KERN_SEMINFO_SEMUSZ, &seminfo.semusz, SYSCTL_INT_READONLY },
> + { KERN_SEMINFO_SEMVMX, &seminfo.semvmx, SYSCTL_INT_READONLY },
> + { KERN_SEMINFO_SEMAEM, &seminfo.semaem, SYSCTL_INT_READONLY },
> { KERN_SEMINFO_SEMOPM, &seminfo.semopm, 1, INT_MAX },
> };
>
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.358
> diff -u -p -r1.358 ip_input.c
> --- netinet/ip_input.c 23 Apr 2021 21:55:36 -0000 1.358
> +++ netinet/ip_input.c 28 Apr 2021 10:04:02 -0000
> @@ -113,7 +113,7 @@ extern int ip_mrtproto;
>
> const struct sysctl_bounded_args ipctl_vars[] = {
> #ifdef MROUTING
> - { IPCTL_MRTPROTO, &ip_mrtproto, 1, 0 },
> + { IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY },
> #endif
> { IPCTL_FORWARDING, &ipforwarding, 0, 2 },
> { IPCTL_SENDREDIRECTS, &ipsendredirects, 0, 1 },
> Index: netinet/tcp_usrreq.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.180
> diff -u -p -r1.180 tcp_usrreq.c
> --- netinet/tcp_usrreq.c 10 Mar 2021 10:21:49 -0000 1.180
> +++ netinet/tcp_usrreq.c 28 Apr 2021 10:04:02 -0000
> @@ -112,7 +112,7 @@ u_int tcp_autorcvbuf_inc = 16 * 1024;
>
> static int pr_slowhz = PR_SLOWHZ;
> const struct sysctl_bounded_args tcpctl_vars[] = {
> - { TCPCTL_SLOWHZ, &pr_slowhz, 1, 0 },
> + { TCPCTL_SLOWHZ, &pr_slowhz, SYSCTL_INT_READONLY },
> { TCPCTL_RFC1323, &tcp_do_rfc1323, 0, 1 },
> { TCPCTL_KEEPINITTIME, &tcptv_keep_init, 1, 3 * TCPTV_KEEP_INIT },
> { TCPCTL_KEEPIDLE, &tcp_keepidle, 1, 5 * TCPTV_KEEP_IDLE },
> Index: netinet6/ip6_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.232
> diff -u -p -r1.232 ip6_input.c
> --- netinet6/ip6_input.c 10 Mar 2021 10:21:49 -0000 1.232
> +++ netinet6/ip6_input.c 28 Apr 2021 10:04:02 -0000
> @@ -1339,9 +1339,9 @@ extern int ip6_mrtproto;
> #endif
>
> const struct sysctl_bounded_args ipv6ctl_vars[] = {
> - { IPV6CTL_DAD_PENDING, &ip6_dad_pending, 1, 0 },
> + { IPV6CTL_DAD_PENDING, &ip6_dad_pending, SYSCTL_INT_READONLY },
> #ifdef MROUTING
> - { IPV6CTL_MRTPROTO, &ip6_mrtproto, 1, 0 },
> + { IPV6CTL_MRTPROTO, &ip6_mrtproto, SYSCTL_INT_READONLY },
> #endif
> { IPV6CTL_FORWARDING, &ip6_forwarding, 0, 1 },
> { IPV6CTL_SENDREDIRECTS, &ip6_sendredirects, 0, 1 },
> Index: sys/sysctl.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/sysctl.h,v
> retrieving revision 1.214
> diff -u -p -r1.214 sysctl.h
> --- sys/sysctl.h 10 Mar 2021 10:21:47 -0000 1.214
> +++ sys/sysctl.h 28 Apr 2021 10:04:02 -0000
> @@ -1000,6 +1000,9 @@ struct sysctl_bounded_args {
> int maximum; /* read-only variable if minimum > maximum */
> };
>
> +#define SYSCTL_INT_UNBOUNDED 0,0
> +#define SYSCTL_INT_READONLY 1,0
> +
> /*
> * Internal sysctl function calling convention:
> *
>
