NetBSD Problem Report #43063

From kre@munnari.OZ.AU  Fri Mar 26 12:26:10 2010
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id C3C0F63B11D
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 26 Mar 2010 12:26:10 +0000 (UTC)
Message-Id: <201003261226.o2QCQ3pk004051@jade.coe.psu.ac.th>
Date: Fri, 26 Mar 2010 19:26:03 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@gnats.NetBSD.org
Subject: Please delete __NetBSD_Prereq__() from <sys/param.h>
X-Send-Pr-Version: 3.95

>Number:         43063
>Category:       kern
>Synopsis:       Please delete __NetBSD_Prereq__() from <sys/param.h>
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 26 12:30:00 +0000 2010
>Last-Modified:  Mon Jul 14 11:55:01 +0000 2014
>Originator:     Robert Elz
>Release:        current (or anything recent)
>Organization:
	Prince of Songkla University
>Environment:
System: NetBSD jade.coe.psu.ac.th 4.0_STABLE NetBSD 4.0_STABLE (JADE-1.696-20080517) #9: Fri May 23 18:55:13 ICT 2008 kre@jade.coe.psu.ac.th:/usr/obj/4/kernels/JADE i386
Architecture: i386
Machine: i386
>Description:
	The __NetBSD_Prereq__() in <sys/param.h> was changed from
	testing __NetBSD_Version__ >= m.n to testing
	__NetBSD_Version <= m.n in version 1.242 of <sys/param.h>

	That is, its sense was entirely reversed.   Which is the
	correct version is irrelevant now (I kind of believe the
	former, but I might be rationalising its intended use based
	upon the definition I first saw).

	The effect is that it is impossible (in sources outside of
	NetBSD's control) for anyone to sanely use this macro, as
	they'd have no way of telling which version they were going
	to get (or at least, not without doing the test that
	the macro is designed to do in another way first...)

	It could still be used inside NetBSD, where its usage could be
	synchronised with the system, but it isn't - that is, I did a
	complete search of all the NetBSD current, and the only reference
	to the macro was the one in sys/param.h

	That is, it is not used the only place it could safely be used.
	If something similar is ever needed, it can be recreated
	with a new name, and a safe to use definition.   For now,
	please delete it, so any external code that might happen to
	use the thing (obviously I can't search all the world's code
	to check) will fail with a compile error (trivially fixed)
	rather than generating an unknowable test, and on some systems
	(the ones where the test is the "wrong" way for the application)
	bizarre results.

>How-To-Repeat:
	By inspection.
>Fix:

ed src/sys/sys/param.h <<!
/__NetBSD_Prereq__/;+2d
w
q
!

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from <sys/param.h>
Date: Fri, 26 Mar 2010 15:06:23 +0100

 We could instead provide both variants as

   __NetBSD_older_than__(maj,min) /* NetBSD older than maj.min */

 and

   __NetBSD_newer_than__(maj,min) /* NetBSD maj.min or newer */

 if there realy is a need for such a test in pkgsrc world. Pkgsrc folks?

 Martin

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from
	<sys/param.h>
Date: Sat, 27 Mar 2010 19:31:11 +0000

 On Fri, Mar 26, 2010 at 02:10:06PM +0000, Martin Husemann wrote:
  >  We could instead provide both variants as
  >  
  >    __NetBSD_older_than__(maj,min) /* NetBSD older than maj.min */
  >  
  >  and
  >  
  >    __NetBSD_newer_than__(maj,min) /* NetBSD maj.min or newer */
  >  
  >  if there realy is a need for such a test in pkgsrc world. Pkgsrc folks?

 In general I don't think there is. But we ought to provide these
 anyway because the alternative is inspecting the bit representation of
 __NetBSD_Version__ and it'd be just as well to avoid/discourage that.

 -- 
 David A. Holland
 dholland@netbsd.org

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from <sys/param.h>
Date: Sun, 28 Mar 2010 16:50:58 +0700

     Date:        Sat, 27 Mar 2010 19:35:02 +0000 (UTC)
     From:        David Holland <dholland-bugs@NetBSD.org>
     Message-ID:  <20100327193502.6F2DF63B11D@www.NetBSD.org>

   |   >  if there realy is a need for such a test in pkgsrc world. Pkgsrc folks?
   |  
   |  In general I don't think there is.

 I certainly don't mind if some new macros are added, but I thought all this
 stuff was for LKMs (originally) and, I guess, for modules now.

 It really makes no sense for pkgsrc (or any application) to test to see what
 kernel version is running - that's useless information, aside from the odd
 ioctl (and similar) for which it is easier to test to see if the ioctl name
 is defined, knowing what kernel is running is of no interest if the userland
 API to the kernel is older - I have run a NetBSD 5 kernel with NetBSD 3.* based
 libc, and NetBSD current (5.99.*) kernels with NetBSD 4 based libc - and
 all of that is supposed to and (some threading issues aside) does work.

 An application that looks at __NetBSD_Version__ from <sys/param.h> and
 decides that means it is 5.99.* and so "just knows" that time_t is 64 bits,
 or whatever else current has that's different from older systems is then
 going to get a shock when time() (and all the time fields in kernel interface
 structs) keep on being 32 bits using the compat interfaces into the kernel.

 I really don't think we ought to be doing anything to encourage pkgsrc
 (or any applications, aside perhaps from a few kernel monitoring type) from
 ever finding out what kernel version happens to be running.

 LKMs are dead, so we don't have to worry about those any more.

 That just leaves modules, and I think the people building those are
 quite able to create a macro to do whatever test on __NetBSD_Version__
 that might need, if they ever need one.

 So, for now, I think I'd suggest just deleting the worse than useless
 __NetBSD_Prereq__ macro, and just wait to see if anything else is needed
 rather than presuming that something will be.

 kre


From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from
	<sys/param.h>
Date: Sun, 28 Mar 2010 19:01:07 +0000

 On Sun, Mar 28, 2010 at 09:55:02AM +0000, Robert Elz wrote:
  > An application that looks at __NetBSD_Version__ from <sys/param.h>
  > and decides that means it is 5.99.* and so "just knows" that time_t
  > is 64 bits, or whatever else current has that's different from
  > older systems is then going to get a shock when time() (and all the
  > time fields in kernel interface structs) keep on being 32 bits
  > using the compat interfaces into the kernel.

 Well... if you install a sys/param.h that says 5.99.* into
 /usr/include (and presumably other kernel headers) and don't rebuild
 libc accordingly, there'll be a lot of shocks, compared to which bad
 version tests are fairly minor.

 Checking what's in sys/param.h (as opposed to calling uname()) is in
 fact the best way to test the version of the userland libs you have...

  >  So, for now, I think I'd suggest just deleting the worse than useless
  >  __NetBSD_Prereq__ macro, and just wait to see if anything else is needed
  >  rather than presuming that something will be.

 Well, I just ran into a case; emacs20 broke on -current because of the
 terminfo merge. It can't probe whether it's using terminfo; it has to
 be told, and so I needed a test for NetBSD < 5.99.24. It turned out to
 be necessary because of unrelated emacs lossage to get the OS version
 from the pkgsrc infrastructure instead of using sys/param.h, but
 sys/param.h would have been the better/simpler approach.

 (Patching in a configure test is the *right* way, but quite a bit too
 involved.)

 -- 
 David A. Holland
 dholland@netbsd.org

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from <sys/param.h>
Date: Mon, 29 Mar 2010 09:33:09 +0700

     Date:        Sun, 28 Mar 2010 19:05:03 +0000 (UTC)
     From:        David Holland <dholland-bugs@NetBSD.org>
     Message-ID:  <20100328190503.6480563B11D@www.NetBSD.org>


   |  Well... if you install a sys/param.h that says 5.99.* into
   |  /usr/include (and presumably other kernel headers)

 I generally don't, not that param.h should do much harm, but types.h
 certainly would, and as you imply, updatig bits of /usr/include/sys
 would be dangerous at best.

   |  Checking what's in sys/param.h (as opposed to calling uname()) is in
   |  fact the best way to test the version of the userland libs you have...

 Perhaps, though I'd have thought /etc/release should be even better.   But
 unfortunately, much build stuff (like anything using autoconf for this kind
 of test, I believe, including pkgsrc's system version tests) doesn't go
 looking in sys/param.h, it does "uname -m" instead - that's exactly why
 libkver works for doing pkg_comp builds (or why it is needed, param.h
 should have the same info, but isn't usually used).

   |  Well, I just ran into a case; emacs20 broke on -current because of the
   |  terminfo merge. It can't probe whether it's using terminfo; it has to
   |  be told, and so I needed a test for NetBSD < 5.99.24.

 You really needed a test whether the terminfo functions are in libc.

   |  (Patching in a configure test is the *right* way, but quite a bit too
   |  involved.)

 Exactly ...   because you're getting the version from pkgsrc, you're
 getting uname output, which will tell you precisely nothing about whether
 or not the system's libc happens to be the version with terminfo in it or
 not...   I could boot such a kernel (briefly) and try building emacs
 (outside of pkg_comp) just to demonstrate it if needed...

 kre


From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from
	<sys/param.h>
Date: Sat, 29 May 2010 19:57:55 +0000

 On Mon, Mar 29, 2010 at 02:35:03AM +0000, Robert Elz wrote:
  >    |  Checking what's in sys/param.h (as opposed to calling uname()) is in
  >    |  fact the best way to test the version of the userland libs you have...
  >  
  >  Perhaps, though I'd have thought /etc/release should be even better.   But
  >  unfortunately, much build stuff (like anything using autoconf for this kind
  >  of test, I believe, including pkgsrc's system version tests) doesn't go
  >  looking in sys/param.h, it does "uname -m" instead - that's exactly why
  >  libkver works for doing pkg_comp builds (or why it is needed, param.h
  >  should have the same info, but isn't usually used).

 Right.

  >    |  (Patching in a configure test is the *right* way, but quite a bit too
  >    |  involved.)
  >  
  >  Exactly ...   because you're getting the version from pkgsrc, you're
  >  getting uname output, which will tell you precisely nothing about whether
  >  or not the system's libc happens to be the version with terminfo in it or
  >  not...   I could boot such a kernel (briefly) and try building emacs
  >  (outside of pkg_comp) just to demonstrate it if needed...

 Yeah, but in this case it was a tradeoff between what's right and
 what's feasible to wedge into an old version of emacs.

 I am sufficiently annoyed with emacs23 that I'm contemplating doing
 some heavier maintenance on emacs20, in which case fixing this up
 properly might be feasible. But that's neither here nor there.

 Anyhow, I like Martin's suggestion.

 -- 
 David A. Holland
 dholland@netbsd.org

From: Roy Marples <roy@marples.name>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete =?UTF-8?Q?=5F=5FNetBSD=5FPrereq=5F?=  =?UTF-8?Q?=5F=28=29=20from=20=3Csys/param=2Eh=3E?=
Date: Sun, 13 Jul 2014 08:34:55 +0100

 Hi

 dhcpcd currently uses this macro like so

 /*
   * BSD kernels don't inform userland of DAD results.
   * See the discussion here:
   *    http://mail-index.netbsd.org/tech-net/2013/03/15/msg004019.html
   */
 #ifndef __linux__
 /* We guard here to avoid breaking a compile on linux ppc-64 headers */
 #  include <sys/param.h>
 #endif
 #ifdef BSD
 #  define IPV6_POLLADDRFLAG
 #endif

 /* This was fixed in NetBSD */
 #ifdef __NetBSD_Prereq__
 #  if __NetBSD_Prereq__(6, 99, 20)
 #    undef IPV6_POLLADDRFLAG
 #  endif
 #endif

 Should this be now be moved to a ./configure test based on uname -m?
 ./configure is NOT run when dhcpcd is imported into external so that may
 complicate future pullups if changed.

 Roy

From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from <sys/param.h>
Date: Sun, 13 Jul 2014 16:15:45 +0200

 On Sun, Jul 13, 2014 at 07:40:01AM +0000, Roy Marples wrote:
 >  /* This was fixed in NetBSD */
 >  #ifdef __NetBSD_Prereq__
 >  #  if __NetBSD_Prereq__(6, 99, 20)
 >  #    undef IPV6_POLLADDRFLAG
 >  #  endif
 >  #endif
 >  
 >  Should this be now be moved to a ./configure test based on uname -m?
 >  ./configure is NOT run when dhcpcd is imported into external so that may
 >  complicate future pullups if changed.

 Just use __NetBSD_Version directly?

 Joerg

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/43063: Please delete __NetBSD_Prereq__() from <sys/param.h>
Date: Sun, 13 Jul 2014 16:55:00 +0000

 On Sun, Jul 13, 2014 at 02:20:01PM +0000, Joerg Sonnenberger wrote:
  >  >  /* This was fixed in NetBSD */
  >  >  #ifdef __NetBSD_Prereq__
  >  >  #  if __NetBSD_Prereq__(6, 99, 20)
  >  >  #    undef IPV6_POLLADDRFLAG
  >  >  #  endif
  >  >  #endif
  >  >  
  >  >  Should this be now be moved to a ./configure test based on uname -m?
  >  >  ./configure is NOT run when dhcpcd is imported into external so that may
  >  >  complicate future pullups if changed.
  >  
  >  Just use __NetBSD_Version directly?

 Anyway, yeah, the problem with __NetBSD_Prereq__ is that you can't
 tell by looking at the name of it whether it means "before 6.99.20" or
 "after 6.99.20", and NetBSD versions (including releases) have been
 shipped both ways.

 So it's much better to write

    #if __NetBSD_Version__ >= 699002000

 although as I noted some years ago, it might be better to avoid
 exposing the bit encoding of the number, something like

    #if __NetBSD_Version__ >= __NetBSD_mkversion__(6, 99, 20)

 The problem with that is that writing a macro that handles all the
 variants (including post-release stable branches and so on) isn't
 entirely trivial.

 The last time I made an attempt to get this settled there wasn't a
 whole lot of opposition to fixing it, but there wasn't a clear
 consensus on how either and so it didn't get done.

 -- 
 David A. Holland
 dholland@netbsd.org

From: Roy Marples <roy@marples.name>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/43063: Please delete =?UTF-8?Q?=5F=5FNetBSD=5FPrereq=5F?=  =?UTF-8?Q?=5F=28=29=20from=20=3Csys/param=2Eh=3E?=
Date: Mon, 14 Jul 2014 12:52:38 +0100

 I have made the suggested change in dhcpcd and imported a new version so 
 dhcpcd no longer uses the __NetBSD_Prereq__ macro.

 Roy

Home	PR Database Search