>Number:         33102
>Category:       kern
>Synopsis:       sysctl(9) suggestions
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 19 08:25:00 +0000 2006
>Originator:     YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
>Release:        NetBSD 3.99.16
>Organization:

>Environment:
	
	
System: NetBSD kaeru 3.99.16 NetBSD 3.99.16 (build.kaeru.xen.nodebug.work) #7: Mon Mar 13 18:41:29 JST 2006 takashi@kaeru:/usr/home/takashi/work/kernel/build.kaeru.xen.nodebug.work i386
Architecture: i386
Machine: i386
>Description:

i have some suggestions related to sysctl helper functions.

1. SYSCTLFN_PROTO/SYSCTLFN_ARGS/SYSCTLFN_CALL are horrible, IMO.
   a structure should be used instead.

   ie.
	typedef struct __sysctlrequest *sysctlrequest_t;
	int sysctl_lookup(sysctlrequest_t);

2. it's better to make sysctl_lookup take a pointer to specify
   where new data is copied in, rather than having a local copy of
   whole "node".

3. abusing const qualifier is a bad idea.
   assuming it's to prevent helper functions from modifying "node", (right?)
   a better way would be hiding the structure definition from
   helper functions, and provide limited accessor functions if
   necessary.  (i think it's possible after the above #2.)

(4. sysctl_lookup is badly named, and confusing with sysctl_locate. :-)


to summarize, an example sysctl helper in sysctl(9) would be:

	static int
	sysctl_helper(sysctlrequest_t req)
	{
		sysctlnode_t node;
		int newvalue;
		int error;

		error = sysctl_lookup(req, &newvalue);
		if (error || !sysctlrequest_modify_p(req)) {
			return error;
		}

		if (newvalue < 0 || newvalue > 20) {
			return EINVAL;
		}

		node = sysctlrequest_node(req);
		sysctlnode_setexternaldata(node, &newvalue);
		return 0;
	}

(sysctlnode_t is a pointer, rather than a structure itself.)

>How-To-Repeat:
	
>Fix:
	

>Unformatted: