On Fri, Apr 30, 2021 at 10:14:31PM -0700, Greg Steuck wrote:
> Vitaliy Makkoveev <m...@openbsd.org> writes:
>
> > On Thu, Apr 29, 2021 at 09:31:57AM -0700, Greg Steuck wrote:
> >> Alexander Bluhm <alexander.bl...@gmx.net> writes:
> >> >> 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.
> >>
> >> Vitaliy, I volunteer to do a separate pass, no reason to gate this clear
> >> improvement.
>
> See below. I tested the diff on amd64, i386 seems trivially correct, but
> if somebody feels like testing (or asking me to spin up a VM), we can do
> it.
Hi, you missing KERN_SYSVMSG, KERN_SYSVSEM, KERN_SYSVSHM variables. The
rest diff is ok by me.
>
> >> > 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
> >>
> >> Unused, maybe introduce a single usage as an example?
> >>
> >
> > Or introduce this with the separate diff which will convert all related
> > structures?
>
> I failed to find a case where to use SYSCTL_INT_UNBOUNDED. We always
> find better "common sense" limits than completely unconstrained.
>
I guess it could be killed.
> From 691cf8f4635f22593fe8319055da7aa340a8164b Mon Sep 17 00:00:00 2001
> From: Greg Steuck <g...@lenny.nest.cx>
> Date: Fri, 30 Apr 2021 21:54:39 -0700
> Subject: [PATCH] Update the remaining SYSCTL_INT_READONLY cases
>
> ---
> sys/arch/amd64/amd64/machdep.c | 8 +++----
> sys/arch/i386/i386/machdep.c | 10 ++++----
> sys/kern/kern_sysctl.c | 44 +++++++++++++++++-----------------
> sys/miscfs/fuse/fuse_vfsops.c | 8 +++----
> sys/ufs/ffs/ffs_vfsops.c | 22 ++++++++---------
> 5 files changed, 46 insertions(+), 46 deletions(-)
>
> diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
> index 40f68fde765..8c0c456da45 100644
> --- a/sys/arch/amd64/amd64/machdep.c
> +++ b/sys/arch/amd64/amd64/machdep.c
> @@ -475,10 +475,10 @@ extern int amd64_has_xcrypt;
> const struct sysctl_bounded_args cpuctl_vars[] = {
> { CPU_LIDACTION, &lid_action, 0, 2 },
> { CPU_PWRACTION, &pwr_action, 0, 2 },
> - { CPU_CPUID, &cpu_id, 1, 0 },
> - { CPU_CPUFEATURE, &cpu_feature, 1, 0 },
> - { CPU_XCRYPT, &amd64_has_xcrypt, 1, 0 },
> - { CPU_INVARIANTTSC, &tsc_is_invariant, 1, 0 },
> + { CPU_CPUID, &cpu_id, SYSCTL_INT_READONLY },
> + { CPU_CPUFEATURE, &cpu_feature, SYSCTL_INT_READONLY },
> + { CPU_XCRYPT, &amd64_has_xcrypt, SYSCTL_INT_READONLY },
> + { CPU_INVARIANTTSC, &tsc_is_invariant, SYSCTL_INT_READONLY },
> };
>
> /*
> diff --git a/sys/arch/i386/i386/machdep.c b/sys/arch/i386/i386/machdep.c
> index 87c5a0ed6a6..a8a45719ff6 100644
> --- a/sys/arch/i386/i386/machdep.c
> +++ b/sys/arch/i386/i386/machdep.c
> @@ -3560,11 +3560,11 @@ idt_vec_free(int vec)
>
> const struct sysctl_bounded_args cpuctl_vars[] = {
> { CPU_LIDACTION, &lid_action, 0, 2 },
> - { CPU_CPUID, &cpu_id, 1, 0 },
> - { CPU_OSFXSR, &i386_use_fxsave, 1, 0 },
> - { CPU_SSE, &i386_has_sse, 1, 0 },
> - { CPU_SSE2, &i386_has_sse2, 1, 0 },
> - { CPU_XCRYPT, &i386_has_xcrypt, 1, 0 },
> + { CPU_CPUID, &cpu_id, SYSCTL_INT_READONLY },
> + { CPU_OSFXSR, &i386_use_fxsave, SYSCTL_INT_READONLY },
> + { CPU_SSE, &i386_has_sse, SYSCTL_INT_READONLY },
> + { CPU_SSE2, &i386_has_sse2, SYSCTL_INT_READONLY },
> + { CPU_XCRYPT, &i386_has_xcrypt, SYSCTL_INT_READONLY },
> };
>
> /*
> diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
> index 2a41db49833..45cd6471247 100644
> --- a/sys/kern/kern_sysctl.c
> +++ b/sys/kern/kern_sysctl.c
> @@ -292,26 +292,26 @@ extern int uvm_wxabort;
> extern int global_ptrace;
>
> const struct sysctl_bounded_args kern_vars[] = {
> - {KERN_OSREV, &openbsd, 1, 0},
> + {KERN_OSREV, &openbsd, SYSCTL_INT_READONLY},
> {KERN_MAXVNODES, &maxvnodes, 0, INT_MAX},
> {KERN_MAXPROC, &maxprocess, 0, INT_MAX},
> {KERN_MAXFILES, &maxfiles, 0, INT_MAX},
> - {KERN_NFILES, &numfiles, 1, 0},
> - {KERN_TTYCOUNT, &tty_count, 1, 0},
> - {KERN_ARGMAX, &arg_max, 1, 0},
> - {KERN_NSELCOLL, &nselcoll, 1, 0},
> - {KERN_POSIX1, &posix_version, 1, 0},
> - {KERN_NGROUPS, &ngroups_max, 1, 0},
> - {KERN_JOB_CONTROL, &int_one, 1, 0},
> - {KERN_SAVED_IDS, &int_one, 1, 0},
> - {KERN_MAXPARTITIONS, &maxpartitions, 1, 0},
> - {KERN_RAWPARTITION, &raw_part, 1, 0},
> + {KERN_NFILES, &numfiles, SYSCTL_INT_READONLY},
> + {KERN_TTYCOUNT, &tty_count, SYSCTL_INT_READONLY},
> + {KERN_ARGMAX, &arg_max, SYSCTL_INT_READONLY},
> + {KERN_NSELCOLL, &nselcoll, SYSCTL_INT_READONLY},
> + {KERN_POSIX1, &posix_version, SYSCTL_INT_READONLY},
> + {KERN_NGROUPS, &ngroups_max, SYSCTL_INT_READONLY},
> + {KERN_JOB_CONTROL, &int_one, SYSCTL_INT_READONLY},
> + {KERN_SAVED_IDS, &int_one, SYSCTL_INT_READONLY},
> + {KERN_MAXPARTITIONS, &maxpartitions, SYSCTL_INT_READONLY},
> + {KERN_RAWPARTITION, &raw_part, SYSCTL_INT_READONLY},
> {KERN_MAXTHREAD, &maxthread, 0, INT_MAX},
> - {KERN_NTHREADS, &nthreads, 1, 0},
> + {KERN_NTHREADS, &nthreads, SYSCTL_INT_READONLY},
> {KERN_SOMAXCONN, &somaxconn, 0, SHRT_MAX},
> {KERN_SOMINCONN, &sominconn, 0, SHRT_MAX},
> {KERN_NOSUIDCOREDUMP, &nosuidcoredump, 0, 3},
> - {KERN_FSYNC, &int_one, 1, 0},
> + {KERN_FSYNC, &int_one, SYSCTL_INT_READONLY},
> {KERN_SYSVMSG,
> #ifdef SYSVMSG
> &int_one,
> @@ -333,13 +333,13 @@ const struct sysctl_bounded_args kern_vars[] = {
> &int_zero,
> #endif
> 1, 0},
> - {KERN_FSCALE, &fscale, 1, 0},
> - {KERN_CCPU, &ccpu, 1, 0},
> - {KERN_NPROCS, &nprocesses, 1, 0},
> + {KERN_FSCALE, &fscale, SYSCTL_INT_READONLY},
> + {KERN_CCPU, &ccpu, SYSCTL_INT_READONLY},
> + {KERN_NPROCS, &nprocesses, SYSCTL_INT_READONLY},
> {KERN_SPLASSERT, &splassert_ctl, 0, 3},
> {KERN_MAXLOCKSPERUID, &maxlocksperuid, 0, INT_MAX},
> {KERN_WXABORT, &uvm_wxabort, 0, 1},
> - {KERN_NETLIVELOCKS, &int_zero, 1, 0},
> + {KERN_NETLIVELOCKS, &int_zero, SYSCTL_INT_READONLY},
> #ifdef PTRACE
> {KERN_GLOBAL_PTRACE, &global_ptrace, 0, 1},
> #endif
> @@ -660,11 +660,11 @@ static int byte_order = BYTE_ORDER;
> static int page_size = PAGE_SIZE;
>
> const struct sysctl_bounded_args hw_vars[] = {
> - {HW_NCPU, &ncpus, 1, 0},
> - {HW_NCPUFOUND, &ncpusfound, 1, 0},
> - {HW_BYTEORDER, &byte_order, 1, 0},
> - {HW_PAGESIZE, &page_size, 1, 0},
> - {HW_DISKCOUNT, &disk_count, 1, 0},
> + {HW_NCPU, &ncpus, SYSCTL_INT_READONLY},
> + {HW_NCPUFOUND, &ncpusfound, SYSCTL_INT_READONLY},
> + {HW_BYTEORDER, &byte_order, SYSCTL_INT_READONLY},
> + {HW_PAGESIZE, &page_size, SYSCTL_INT_READONLY},
> + {HW_DISKCOUNT, &disk_count, SYSCTL_INT_READONLY},
> };
>
> int
> diff --git a/sys/miscfs/fuse/fuse_vfsops.c b/sys/miscfs/fuse/fuse_vfsops.c
> index 3e2af8960af..97f08d6fe10 100644
> --- a/sys/miscfs/fuse/fuse_vfsops.c
> +++ b/sys/miscfs/fuse/fuse_vfsops.c
> @@ -356,10 +356,10 @@ fusefs_init(struct vfsconf *vfc)
> extern int stat_fbufs_in, stat_fbufs_wait, stat_opened_fusedev;
>
> const struct sysctl_bounded_args fusefs_vars[] = {
> - { FUSEFS_OPENDEVS, &stat_opened_fusedev, 1, 0 },
> - { FUSEFS_INFBUFS, &stat_fbufs_in, 1, 0 },
> - { FUSEFS_WAITFBUFS, &stat_fbufs_wait, 1, 0 },
> - { FUSEFS_POOL_NBPAGES, &fusefs_fbuf_pool.pr_npages, 1, 0 },
> + { FUSEFS_OPENDEVS, &stat_opened_fusedev, SYSCTL_INT_READONLY },
> + { FUSEFS_INFBUFS, &stat_fbufs_in, SYSCTL_INT_READONLY },
> + { FUSEFS_WAITFBUFS, &stat_fbufs_wait, SYSCTL_INT_READONLY },
> + { FUSEFS_POOL_NBPAGES, &fusefs_fbuf_pool.pr_npages, SYSCTL_INT_READONLY
> },
> };
>
> int
> diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
> index bd488ef4773..69cabec2e98 100644
> --- a/sys/ufs/ffs/ffs_vfsops.c
> +++ b/sys/ufs/ffs/ffs_vfsops.c
> @@ -1578,21 +1578,21 @@ const struct sysctl_bounded_args ffs_vars[] = {
> #ifdef FFS_SOFTUPDATES
> { FFS_MAX_SOFTDEPS, &max_softdeps, 0, INT_MAX },
> { FFS_SD_TICKDELAY, &tickdelay, 2, INT_MAX },
> - { FFS_SD_WORKLIST_PUSH, &stat_worklist_push, 1, 0 }, /* read-only */
> - { FFS_SD_BLK_LIMIT_PUSH, &stat_blk_limit_push, 1, 0 },
> - { FFS_SD_INO_LIMIT_PUSH, &stat_ino_limit_push, 1, 0 },
> - { FFS_SD_BLK_LIMIT_HIT, &stat_blk_limit_hit, 1, 0 },
> - { FFS_SD_INO_LIMIT_HIT, &stat_ino_limit_hit, 1, 0 },
> - { FFS_SD_SYNC_LIMIT_HIT, &stat_sync_limit_hit, 1, 0 },
> - { FFS_SD_INDIR_BLK_PTRS, &stat_indir_blk_ptrs, 1, 0 },
> - { FFS_SD_INODE_BITMAP, &stat_inode_bitmap, 1, 0 },
> - { FFS_SD_DIRECT_BLK_PTRS, &stat_direct_blk_ptrs, 1, 0 },
> - { FFS_SD_DIR_ENTRY, &stat_dir_entry, 1, 0 },
> + { FFS_SD_WORKLIST_PUSH, &stat_worklist_push, SYSCTL_INT_READONLY },
> + { FFS_SD_BLK_LIMIT_PUSH, &stat_blk_limit_push, SYSCTL_INT_READONLY },
> + { FFS_SD_INO_LIMIT_PUSH, &stat_ino_limit_push, SYSCTL_INT_READONLY },
> + { FFS_SD_BLK_LIMIT_HIT, &stat_blk_limit_hit, SYSCTL_INT_READONLY },
> + { FFS_SD_INO_LIMIT_HIT, &stat_ino_limit_hit, SYSCTL_INT_READONLY },
> + { FFS_SD_SYNC_LIMIT_HIT, &stat_sync_limit_hit, SYSCTL_INT_READONLY },
> + { FFS_SD_INDIR_BLK_PTRS, &stat_indir_blk_ptrs, SYSCTL_INT_READONLY },
> + { FFS_SD_INODE_BITMAP, &stat_inode_bitmap, SYSCTL_INT_READONLY },
> + { FFS_SD_DIRECT_BLK_PTRS, &stat_direct_blk_ptrs, SYSCTL_INT_READONLY },
> + { FFS_SD_DIR_ENTRY, &stat_dir_entry, SYSCTL_INT_READONLY },
> #endif
> #ifdef UFS_DIRHASH
> { FFS_DIRHASH_DIRSIZE, &ufs_mindirhashsize, 0, INT_MAX },
> { FFS_DIRHASH_MAXMEM, &ufs_dirhashmaxmem, 0, INT_MAX },
> - { FFS_DIRHASH_MEM, &ufs_dirhashmem, 1, 0 },
> + { FFS_DIRHASH_MEM, &ufs_dirhashmem, SYSCTL_INT_READONLY },
> #endif
> };
>
> --
> 2.31.1
>
