Re: bin/43587: stupid error message during boot from sysctl if no COMPAT40 in

[Wolfgang Stukenbrock <Wolfgang.Stukenbrock%nagler-company.com@localhost>]

The following reply was made to PR bin/43587; it has been noted by GNATS.

From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock%nagler-company.com@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: gnats-admin%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost,
        Wolfgang.Stukenbrock%nagler-company.com@localhost
Subject: Re: bin/43587: stupid error message during boot from sysctl if no 
COMPAT40 in
Date: Fri, 09 Jul 2010 14:18:07 +0200

 Hi,
 
 not 100% OK, but if you replace the line
 "if (memcmp(&o, &i, so) == 0)"
 in write_number() with
 "if (memcmp(o, i, so) == 0)"
 it works.
 
 o and i are of type void*, and it makes no sence to compare the pointers.
 The string part has not been tested by me, but it looks good to me.
 
 The missing free() you've added should go into the distribution in any case.
 
 By the way: your version fo sysctl.c seems to be more recent than the 
 version in 5.0.2.
 I've added and tested the patch to the 5.0.2 version.
 
 I think this should go to the next available distribution.
 
 Thanks.
 
 W. Stukenbrock
 
 
 
 Martin Husemann wrote:
 
 > The following reply was made to PR bin/43587; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin%duskware.de@localhost>
 > To: gnats-bugs%NetBSD.org@localhost
 > Cc: 
 > Subject: Re: bin/43587: stupid error message during boot from sysctl if no 
 > COMPAT40 in 
 > Date: Fri, 9 Jul 2010 13:30:46 +0200
 > 
 >  --Nq2Wo0NMKNjxTN9z
 >  Content-Type: text/plain; charset=us-ascii
 >  Content-Disposition: inline
 >  
 >  Untested, but maybe something along the lines of the attached patch would do
 >  it?
 >  
 >  The idea is to avoid the "try changing the value" part if the old value
 >  is what should be set anyway.
 >  
 >  Martin
 >  
 >  --Nq2Wo0NMKNjxTN9z
 >  Content-Type: text/plain; charset=us-ascii
 >  Content-Disposition: attachment; filename=patch
 >  
 >  Index: sysctl.c
 >  ===================================================================
 >  RCS file: /cvsroot/src/sbin/sysctl/sysctl.c,v
 >  retrieving revision 1.131
 >  diff -u -p -r1.131 sysctl.c
 >  --- sysctl.c        11 Apr 2010 01:52:10 -0000      1.131
 >  +++ sysctl.c        9 Jul 2010 11:28:33 -0000
 >  @@ -125,8 +125,8 @@ static const struct handlespec *findhand
 >   static void canonicalize(const char *, char *);
 >   static void purge_tree(struct sysctlnode *);
 >   static void print_tree(int *, u_int, struct sysctlnode *, u_int, int);
 >  -static void write_number(int *, u_int, struct sysctlnode *, char *);
 >  -static void write_string(int *, u_int, struct sysctlnode *, char *);
 >  +static void write_number(int *, u_int, struct sysctlnode *, char *, bool);
 >  +static void write_string(int *, u_int, struct sysctlnode *, char *, bool);
 >   static void display_number(const struct sysctlnode *, const char *,
 >                         const void *, size_t, int);
 >   static void display_string(const struct sysctlnode *, const char *,
 >  @@ -898,10 +898,10 @@ parse(char *l)
 >      case CTLTYPE_INT:
 >      case CTLTYPE_BOOL:
 >      case CTLTYPE_QUAD:
 >  -           write_number(&name[0], namelen, node, value);
 >  +           write_number(&name[0], namelen, node, value, optional);
 >              break;
 >      case CTLTYPE_STRING:
 >  -           write_string(&name[0], namelen, node, value);
 >  +           write_string(&name[0], namelen, node, value, optional);
 >              break;
 >      case CTLTYPE_STRUCT:
 >              /*
 >  @@ -1697,7 +1697,8 @@ sysctlperror(const char *fmt, ...)
 >    * ********************************************************************
 >    */
 >   static void
 >  -write_number(int *name, u_int namelen, struct sysctlnode *node, char 
 > *value)
 >  +write_number(int *name, u_int namelen, struct sysctlnode *node, char 
 > *value,
 >  +   bool optional)
 >   {
 >      u_int ii, io;
 >      u_quad_t qi, qo;
 >  @@ -1752,6 +1753,15 @@ write_number(int *name, u_int namelen, s
 >              break;
 >      }
 >   
 >  +   if (optional) {
 >  +           rc = sysctl(name, namelen, o, &so, NULL, 0);
 >  +           if (rc == -1) {
 >  +                   sysctlerror(0);
 >  +                   return;
 >  +           }
 >  +           if (memcmp(&o, &i, so) == 0)
 >  +                   return;
 >  +   }
 >      rc = sysctl(name, namelen, o, &so, i, si);
 >      if (rc == -1) {
 >              sysctlerror(0);
 >  @@ -1775,7 +1785,8 @@ write_number(int *name, u_int namelen, s
 >   }
 >   
 >   static void
 >  -write_string(int *name, u_int namelen, struct sysctlnode *node, char 
 > *value)
 >  +write_string(int *name, u_int namelen, struct sysctlnode *node, char 
 > *value,
 >  +   bool optional)
 >   {
 >      char *i, *o;
 >      size_t si, so;
 >  @@ -1794,8 +1805,21 @@ write_string(int *name, u_int namelen, s
 >              exit(1);
 >      }
 >   
 >  +   if (optional) {
 >  +           rc = sysctl(name, namelen, o, &so, NULL, 0);
 >  +           if (rc == -1) {
 >  +                   free(o);
 >  +                   sysctlerror(0);
 >  +                   return;
 >  +           }
 >  +           if (strcmp(o, i) == 0) {
 >  +                   free(o);
 >  +                   return;
 >  +           }
 >  +   }
 >      rc = sysctl(name, namelen, o, &so, i, si);
 >      if (rc == -1) {
 >  +           free(o);
 >              sysctlerror(0);
 >              return;
 >      }
 >  
 >  --Nq2Wo0NMKNjxTN9z--
 >  
 >