NetBSD Problem Report #45935

From kre@munnari.OZ.AU  Mon Feb  6 12:25:30 2012
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id E0E0663B86B
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  6 Feb 2012 12:25:29 +0000 (UTC)
Message-Id: <201202061225.q16CPHeJ027254@jade.coe.psu.ac.th>
Date: Mon, 6 Feb 2012 19:25:17 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@gnats.NetBSD.org
Subject: Requested change to comments in sys/param.h
X-Send-Pr-Version: 3.95

>Number:         45935
>Category:       kern
>Synopsis:       Requested change to comments in sys/param.h
>Confidential:   no
>Severity:       non-critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Mon Feb 06 12:30:01 +0000 2012
>Last-Modified:  Wed Feb 08 13:20:02 +0000 2012
>Originator:     Robert Elz
>Release:        NetBSD current (patch to 5.99.64 included) but irrelevant...
>Organization:
	Prince of Songkla University
>Environment:
System: NetBSD jade.coe.psu.ac.th 5.1_STABLE NetBSD 5.1_STABLE (JADE-1.12-20120130) #27: Tue Jan 31 05:20:31 ICT 2012 kre@jade.coe.psu.ac.th:/usr/obj/5/kernels/i386/JADE i386
Architecture: i386
Machine: i386
>Description:
	The comments in sys/param.h no longer accurately reflect
	the way the __NetBSD_Version__ macro is defined (as illustrated
	by the historical record).

	On tech-kern a while ago, I asked for a clarification of the
	rules for use of __NetBSD_Version__ and received no reply
	(which suggests that no-one actually knows).

	Aside: there were replies on a few side issues, but no reply that
		actually said "this is the policy" or "the policy can
		be found in ..."

	Hence, I am proposing some rules that seem to match the way the
	macro has been used in the (relatively) recent past - that is,
	no-one cases about 1.6ZA any more, and I supply here a patch to
	param.h that documents, in comments (the patch is almost exclusively
	to comments) my suggested rules.

	By all means make a different change if the rules I'm suggesting
	aren't what ought to be used, but please make some change so
	everyone knows just how __NetBSD_Version__ is intended to be used
	(it seems fairly well understood for NetBSD-current, and barely
	comprehended elsewhere).

	If the preference is to keep param.h smaller, then remove all of
	the explanation from there (the current incorrect version, and
	my suggested change), but do put it somewhere else.

>How-To-Repeat:
	By inspection of sys/param.h along with a grep for __NetBSD_Version__
	in the master cvs file to see the various values it has taken over
	the years (I'm not going to waste space by including that here).

>Fix:
	My suggested patch is appended.   Apply this, or something else.

	Note: this patch also removes __NetBSD_Prereq__() which everyone
	seems agree should go, but doesn't replace it with anything
	along the lines David Holland suggested - by all means defer that
	change until he is ready to make it, this is just my nudge/vote!
	(That was the "almost" in the "almost exclusively to comments" above)

	Also note: in patch format some (well, one) of the lines below extend
	(just) beyond 80 columns, but if incorporated into param.h, nothing
	goes wider than column 80.

	I'd actually like to change the definition of __NetBSD_Version__
	to be (with appropriate numbers in place of M m and p of course):
		#define __NetBSD_Version__ (__NetBSD_VersID__(M,m,p)) /* cmt */

	and add
		#define	__NetBSD_VersID__(M,m,p) \
			(((M) * 100000000) + \
			 ((m) * 1000000) + \
			 ((p) * 100))

	and when z (see the patch) != 0
		#define	__NetBSD_Version__ (__NetBSD_VersID__(M,m,p)+z)

	But I thought that too much of a variation for this change request,
	which is really just trying to clarify the rules.   Building the
	version this way would avoid the error that occurred with 3.0.1 (etc)
	where the "1" was inserted into the wrong column, making the
	version in __NetBSD_Version__ meaning 3.0.10 where the comment said
	3.0.1 (and the latter was clearly intended).

--- /release/current/usr/src/sys/sys/param.h	2012-02-05 21:28:51.000000000 +0700
+++ param.h	2012-02-06 19:12:30.000000000 +0700
@@ -47,12 +47,73 @@
 #define	BSD4_4	1

 /*
- *	#define __NetBSD_Version__ MMmmrrpp00
+ *	#define __NetBSD_Version__ MMmmrrppzz
  *
  *	M = major version
  *	m = minor version; a minor number of 99 indicates current.
  *	r = 0 (*)
  *	p = patchlevel
+ *	z = trivia discriminator
+ *
+ * __NetBSD_Version__ is formed: ((M*100000000)+(m*1000000)+(r*10000)+(p*100)+z)
+ *
+ *	0 <= M < 42	(by the time this is a problem, the restriction will go)
+ *	0 <= m <= 99
+ *	r = 0		(field not currently used)
+ *	m == 99 ? 0 <= p <= 9999 : 0 <= p <= 99
+ *	0 <= z <= 99
+ *
+ * For NetBSD-current  (developing next major release which will be (M+1).0)
+ *	m == 99
+ *	p initially 0, then incremented for every internal ABI change
+ *	z is always 0
+ *
+ * For releases and other "stable" versions
+ *	M == major release number (remains constant for a branch)
+ *		for new major releases, incr(M); m=0; p=0; z=0
+ *	m == minor release number
+ *		releases with new functionality: incr(m); p=0; z=0
+ *	p == patchlevel indicator
+ *		security and serious fixes only, each new release incr(p); z=0
+ *	z == version bump counter
+ *		incremented (from 0) if any change is included that would
+ *		result in a different release set (even a typo in a comment...)
+ *		from any previously released, or almost released, version.
+ *		(A version is "almost released" once it is tagged in CVS).
+ *
+ * __NetBSD_Version__ must be altered for all ABI changes in -current
+ *		(multiple ABI changes with one increase in p is OK)
+ *	and for every new release that is made for patch/feature releases.
+ *
+ * Note: The comment on the #define of __NetBSD_Version__ is important
+ * and must also be maintained correctly:
+ *
+ * In released versions (anything prepared as binary sets for distribution)
+ *		NetBSD M.m		(if p == 0)
+ * or
+ *		NetBSD M.m.p		(if p != 0)
+ * In release candidates (pre-release test versions) it will be
+ *		NetBSD M.m_ZZZ
+ *	where "ZZZ" is "ALPHA", "BETA" or "RC"; often with a number appended.
+ *	In release candidates __NetBSD_Version__ should be set to the value
+ *	for the release being prepared.  If changes are required, other than
+ *	to the  __NetBSD_Version__ definition line itself, then incr(z) for
+ *	the next test release or the final release.  There are no release
+ *	candidates for minor (point) releases - hence no need for M.m.p_ZZZ.
+ * In NetBSD-current the comment should say
+ *		NetBSD M.99.p
+ * In major branches (between releases) it should say
+ *		NetBSD M.m_STABLE
+ * In minor branches (between releases) it should say
+ *		NetBSD M.m.p_PATCH
+ *	for the latter 2 cases, __NetBSD_Version__ will remain at the last
+ *	released value for the appropriate branch between releases.
+ *	These versions should only be seen by end users who checkout from CVS.
+ *
+ * No (unnecessary) leading 0's in numeric values in the comment, that is:
+ *	use NetBSD 5.99.1 not NetBSD 5.99.01 and NetBSD 5.0.1 not NetBSD 5.0.01
+ *
+ * Nothing else is permitted in the comment on the #define.
  *
  * When new releases are made, src/gnu/usr.bin/groff/tmac/mdoc.local
  * needs to be updated and the changes sent back to the groff maintainers.
@@ -65,9 +126,6 @@

 #define	__NetBSD_Version__	599006400	/* NetBSD 5.99.64 */

-#define __NetBSD_Prereq__(M,m,p) (((((M) * 100000000) + \
-    (m) * 1000000) + (p) * 100) <= __NetBSD_Version__)
-
 /*
  * Historical NetBSD #define
  *

>Audit-Trail:
From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/45935: Requested change to comments in sys/param.h
Date: Mon, 6 Feb 2012 19:50:32 +0200

 On Mon, 06 Feb 2012, kre@munnari.OZ.AU wrote:
 >	Hence, I am proposing some rules that seem to match the way the
 >	macro has been used in the (relatively) recent past - that is,
 >	no-one cases about 1.6ZA any more, and I supply here a patch to
 >	param.h that documents, in comments (the patch is almost exclusively
 >	to comments) my suggested rules.

 Thank you.

 >	Note: this patch also removes __NetBSD_Prereq__()

 I'd prefer to leave that for a separate change.

 >	I'd actually like to change the definition of __NetBSD_Version__
 >	to be (with appropriate numbers in place of M m and p of course):
 >		#define __NetBSD_Version__ (__NetBSD_VersID__(M,m,p)) /* cmt */

 I like that, but will scripts that search for the comment be able to
 handle it?

 >+ *	0 <= M < 42	(by the time this is a problem, the restriction will go)
 >+ *	0 <= m <= 99
 >+ *	r = 0		(field not currently used)
 >+ *	m == 99 ? 0 <= p <= 9999 : 0 <= p <= 99
 >+ *	0 <= z <= 99

 Only two digits are reserved for "pp", so I think it should
 be restricted to [0..99].  Alternatively, remove "rr", and
 let "pppp" be 4 digits.

 --apb (Alan Barrett)

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/45935: Requested change to comments in sys/param.h
Date: Tue, 07 Feb 2012 01:31:42 +0700

     Date:        Mon,  6 Feb 2012 17:55:03 +0000 (UTC)
     From:        Alan Barrett <apb@cequrux.com>
     Message-ID:  <20120206175503.0B45963B86B@www.NetBSD.org>

   |  >	Note: this patch also removes __NetBSD_Prereq__()
   |  I'd prefer to leave that for a separate change.

 I expected that...

   |  I like that, but will scripts that search for the comment be able to
   |  handle it?

 Not sure, nor am I sure what all the scripts are.   The one I use for
 kernel building which uses that value to construct the output directory
 name would not have a problem, it looks for the ' NetBSD ' in the comment
 and ignores everything before that.   But others, I'm not sure - and in
 any case, I think that should also be a separate change.

   |  Only two digits are reserved for "pp", so I think it should
   |  be restricted to [0..99].  Alternatively, remove "rr", and
   |  let "pppp" be 4 digits.

 It works either way, I just didn't want anyone to panic upon nearing
 5.99.99 and to be able to just continue on to 5.99.100 (and so on).
 It all works whether the 1 is considered as an unusual value in the r
 field or just a wider p field.   On the other hand, I can't even
 imagine the need for a 5.0.100 release (or even 5.0.10), so for everything
 other than current, 2 digits is really overkill already.

 But as I said, whatever works, just so there is some documented method
 that actually matches reality (which includes not always having 0's in the
 final 2 digit positions.)

 kre

From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/45935: Requested change to comments in sys/param.h
Date: Wed, 8 Feb 2012 15:19:04 +0200

 On Mon, 06 Feb 2012, kre@munnari.OZ.AU wrote:
 >+ *	#define __NetBSD_Version__ MMmmrrppzz
 >  *
 >  *	M = major version
 >  *	m = minor version; a minor number of 99 indicates current.
 >  *	r = 0 (*)
 >  *	p = patchlevel
 >+ *	z = trivia discriminator
     [...]
 >+ * For releases and other "stable" versions
 >+ *	z == version bump counter
 >+ *		incremented (from 0) if any change is included that would
 >+ *		result in a different release set (even a typo in a comment...)
 >+ *		from any previously released, or almost released, version.
 >+ *		(A version is "almost released" once it is tagged in CVS).

 We haven't consistently incremented the zz digits at the end of
 __NetBSD_Version__.  For example, we had:

 #define __NetBSD_Version__      400000000       /* NetBSD 4.0_BETA */
 #define __NetBSD_Version__      400000001       /* NetBSD 4.0_BETA2 */
 #define __NetBSD_Version__      400000002       /* NetBSD 4.0_BETA2 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_RC1 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_BETA2 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_RC1 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_RC2 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_RC3 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_RC4 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0_RC5 */
 #define __NetBSD_Version__      400000003       /* NetBSD 4.0 */

 #define __NetBSD_Version__      501000000       /* NetBSD 5.1 */
 #define __NetBSD_Version__      501000000       /* NetBSD 5.1_RC1 */
 #define __NetBSD_Version__      501000000       /* NetBSD 5.1_RC2 */
 #define __NetBSD_Version__      501000000       /* NetBSD 5.1_RC3 */
 #define __NetBSD_Version__      501000000       /* NetBSD 5.1_RC4 */
 #define __NetBSD_Version__      501000000       /* NetBSD 5.1.0_PATCH */

 I think that similar changes in future should all get different zz digits.

 --apb (Alan Barrett)

Home	PR Database Search