Subject: Re: CVS commit: syssrc/sys/kern
To: Simon Burge <simonb@wasabisystems.com>
From: enami tsugutomo <enami@but-b.or.jp>
List: source-changes
Date: 11/03/2002 11:31:56
> The way I had intended this sysctls to work was if the structure size
> mib entry wasn't passed in then we use the current kernel size and that
> this mib entry was optional.  I guess this really only matters for
> source-code compatibility with older programs that aren't written with
> forward compatibility in mind.
> 
> This leaves us with a choice - do we make the size mib entry mandatory
> now?

IMHO, using the sysctl without passing the size of struct should be
discouraged.  Such source code just generates backward incompatible
binary.  I guess we could do something like below, but I'd like to
just return EINVAL :-).

Anyway, we should keep 1.6 iostat etc to run with -current kernel.

enami.
Index: sys/sysctl.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/sysctl.h,v
retrieving revision 1.78
diff -c -r1.78 sysctl.h
*** sys/sysctl.h	2002/08/26 13:09:39	1.78
--- sys/sysctl.h	2002/11/03 02:11:15
***************
*** 474,484 ****
  #define	HW_USERMEM	 6		/* int: non-kernel memory */
  #define	HW_PAGESIZE	 7		/* int: software page size */
  #define	HW_DISKNAMES	 8		/* string: disk drive names */
! #define	HW_DISKSTATS	 9		/* struct: diskstats[] */
  #define	HW_MACHINE_ARCH	10		/* string: machine architecture */
  #define	HW_ALIGNBYTES	11		/* int: ALIGNBYTES for the kernel */
  #define	HW_CNMAGIC	12		/* string: console magic sequence(s) */
! #define	HW_MAXID	13		/* number of valid hw ids */
  
  #define	CTL_HW_NAMES { \
  	{ 0, 0 }, \
--- 474,485 ----
  #define	HW_USERMEM	 6		/* int: non-kernel memory */
  #define	HW_PAGESIZE	 7		/* int: software page size */
  #define	HW_DISKNAMES	 8		/* string: disk drive names */
! #define	HW_ODISKSTATS	 9		/* struct: old diskstats[] */
  #define	HW_MACHINE_ARCH	10		/* string: machine architecture */
  #define	HW_ALIGNBYTES	11		/* int: ALIGNBYTES for the kernel */
  #define	HW_CNMAGIC	12		/* string: console magic sequence(s) */
! #define	HW_DISKSTATS	13		/* struct: diskstats[] */
! #define	HW_MAXID	14		/* number of valid hw ids */
  
  #define	CTL_HW_NAMES { \
  	{ 0, 0 }, \
***************
*** 490,499 ****
  	{ "usermem", CTLTYPE_INT }, \
  	{ "pagesize", CTLTYPE_INT }, \
  	{ "disknames", CTLTYPE_STRING }, \
! 	{ "diskstats", CTLTYPE_STRUCT }, \
  	{ "machine_arch", CTLTYPE_STRING }, \
  	{ "alignbytes", CTLTYPE_INT }, \
  	{ "cnmagic", CTLTYPE_STRING }, \
  }
  
  /*
--- 491,501 ----
  	{ "usermem", CTLTYPE_INT }, \
  	{ "pagesize", CTLTYPE_INT }, \
  	{ "disknames", CTLTYPE_STRING }, \
! 	{ "odiskstats", CTLTYPE_STRUCT }, \
  	{ "machine_arch", CTLTYPE_STRING }, \
  	{ "alignbytes", CTLTYPE_INT }, \
  	{ "cnmagic", CTLTYPE_STRING }, \
+ 	{ "diskstats", CTLTYPE_STRUCT }, \
  }
  
  /*
***************
*** 694,700 ****
  int sysctl_rdminstruct(void *, size_t *, void *, const void *, size_t);
  int sysctl_clockrate(void *, size_t *);
  int sysctl_disknames(void *, size_t *);
! int sysctl_diskstats(int *, u_int, void *, size_t *);
  int sysctl_vnode(char *, size_t *, struct proc *);
  int sysctl_ntptime(void *, size_t *);
  #ifdef GPROF
--- 696,702 ----
  int sysctl_rdminstruct(void *, size_t *, void *, const void *, size_t);
  int sysctl_clockrate(void *, size_t *);
  int sysctl_disknames(void *, size_t *);
! int sysctl_diskstats(int *, u_int, void *, size_t *, struct proc *);
  int sysctl_vnode(char *, size_t *, struct proc *);
  int sysctl_ntptime(void *, size_t *);
  #ifdef GPROF
Index: kern/subr_disk.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/subr_disk.c,v
retrieving revision 1.46
diff -c -r1.46 subr_disk.c
*** kern/subr_disk.c	2002/11/01 15:20:03	1.46
--- kern/subr_disk.c	2002/11/03 02:11:16
***************
*** 387,394 ****
  	return (error);
  }
  
  int
! sysctl_diskstats(int *name, u_int namelen, void *vwhere, size_t *sizep)
  {
  	struct disk_sysctl sdisk;
  	struct disk *diskp;
--- 387,411 ----
  	return (error);
  }
  
+ /* The old struct disk_sysctl used in NetBSD 1.6 */
+ struct odisk_sysctl {
+ 	char		dk_name[DK_DISKNAMELEN];
+ 	int32_t		dk_busy;
+ 	int32_t		pad;
+ 	u_int64_t	dk_xfer;
+ 	u_int64_t	dk_seek;
+ 	u_int64_t	dk_bytes;
+ 	u_int32_t	dk_attachtime_sec;
+ 	u_int32_t	dk_attachtime_usec;
+ 	u_int32_t	dk_timestamp_sec;
+ 	u_int32_t	dk_timestamp_usec;
+ 	u_int32_t	dk_time_sec;
+ 	u_int32_t	dk_time_usec;
+ };
+ 
  int
! sysctl_diskstats(int *name, u_int namelen, void *vwhere, size_t *sizep,
!     struct proc *p)
  {
  	struct disk_sysctl sdisk;
  	struct disk *diskp;
***************
*** 396,413 ****
  	size_t tocopy, left;
  	int error;
  
  	if (where == NULL) {
! 		if (namelen == 0)
! 			*sizep = disk_count * sizeof(sdisk);
! 		else
! 			*sizep = disk_count * name[0];
  		return (0);
  	}
- 
- 	if (namelen == 0)
- 		tocopy = sizeof(sdisk);
- 	else
- 		tocopy = name[0];
  
  	error = 0;
  	left = *sizep;
--- 413,435 ----
  	size_t tocopy, left;
  	int error;
  
+ 	if (namelen <= 1) {
+ 		if (name[0] == HW_ODISKSTATS)
+ 			tocopy = sizeof(struct odisk_sysctl);
+ 		else {
+ 			tocopy = sizeof(struct disk_sysctl);
+ 			log(LOG_WARNING, "pid %d (%s): "
+ 			    "sysctl(HW_DISKSTATS) used without "
+ 			    "structure size\n",
+ 			    p->p_pid, p->p_comm);
+ 		}
+ 	} else
+ 		tocopy = name[1];
+ 
  	if (where == NULL) {
! 		*sizep = disk_count * tocopy;
  		return (0);
  	}
  
  	error = 0;
  	left = *sizep;
Index: kern/kern_sysctl.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/kern_sysctl.c,v
retrieving revision 1.113
diff -c -r1.113 kern_sysctl.c
*** kern/kern_sysctl.c	2002/09/04 01:32:40	1.113
--- kern/kern_sysctl.c	2002/11/03 02:11:18
***************
*** 602,607 ****
--- 603,609 ----
  
  	/* All sysctl names at this level, except for a few, are terminal. */
  	switch (name[0]) {
+ 	case HW_ODISKSTATS:
  	case HW_DISKSTATS:
  		/* Not terminal. */
  		break;
***************
*** 632,639 ****
  		return (sysctl_rdint(oldp, oldlenp, newp, ALIGNBYTES));
  	case HW_DISKNAMES:
  		return (sysctl_disknames(oldp, oldlenp));
  	case HW_DISKSTATS:
! 		return (sysctl_diskstats(name + 1, namelen - 1, oldp, oldlenp));
  	case HW_CNMAGIC: {
  		char magic[CNS_LEN];
  		int error;
--- 634,642 ----
  		return (sysctl_rdint(oldp, oldlenp, newp, ALIGNBYTES));
  	case HW_DISKNAMES:
  		return (sysctl_disknames(oldp, oldlenp));
+ 	case HW_ODISKSTATS:
  	case HW_DISKSTATS:
! 		return (sysctl_diskstats(name, namelen, oldp, oldlenp, p));
  	case HW_CNMAGIC: {
  		char magic[CNS_LEN];
  		int error;