Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Jun 2013 10:24:16 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        David Chisnall <theraven@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r251855 - head/sys/sys
Message-ID:  <20130618093254.L1323@besplex.bde.org>
In-Reply-To: <201306171530.r5HFUmx5035189@svn.freebsd.org>
References:  <201306171530.r5HFUmx5035189@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 17 Jun 2013, David Chisnall wrote:

> Log:
>  Rename a parameter in sys/time.h so that you don't get warnings for things
>  like libdialog that include both this header and math.h.

All the bintime stuff has similar namespace errors, starting with the
field names 'sec' and 'frac' not having a prefix, despite the good example
set by struct tm and good style requiring a prefix.  'sec' and 'frac' are
paticularly bad field names since they are close to both English words
and to good names for application variables.

> Modified: head/sys/sys/time.h
> ==============================================================================
> --- head/sys/sys/time.h	Mon Jun 17 15:16:14 2013	(r251854)
> +++ head/sys/sys/time.h	Mon Jun 17 15:30:47 2013	(r251855)
> @@ -103,17 +103,17 @@ bintime_mul(struct bintime *bt, u_int x)
> }
>
> static __inline void
> -bintime_shift(struct bintime *bt, int exp)
> +bintime_shift(struct bintime *__bt, int __exp)

The new names have excessive underscores and are thus now just style
bugs.  Single underscores are ugly enough.

'exp' us only reserved by system headers when <math.h> is included, and
even then it is not normally a problem since it is not a macro.

But bt is in the application namespace.

#define	bt	bintime variables should be named _bt in inline functions
#include <sys/time.h>

This is now fixed, but only for this function.

> {
>
> -	if (exp > 0) {
> -		bt->sec <<= exp;
> -		bt->sec |= bt->frac >> (64 - exp);
> -		bt->frac <<= exp;
> -	} else if (exp < 0) {
> -		bt->frac >>= -exp;
> -		bt->frac |= (uint64_t)bt->sec << (64 + exp);
> -		bt->sec >>= -exp;
> +	if (__exp > 0) {
> +		__bt->sec <<= __exp;
> +		__bt->sec |= __bt->frac >> (64 - __exp);
> +		__bt->frac <<= __exp;
> +	} else if (__exp < 0) {
> +		__bt->frac >>= -__exp;
> +		__bt->frac |= (uint64_t)__bt->sec << (64 + __exp);
> +		__bt->sec >>= -__exp;
> 	}
> }

__bt has another naming error.  It is not a bintime variable, but a pointer
to one.  It should be named _btp.  This style bug is everywhere dense in
bintime code.

'sec' and 'frac' are in the application namespace.

#define	sec	namespace errors in bintime bite me if I do this
#define	frac	the errors are just as undocumented as the namespace
#include <sys/time.h>

This is not fixed.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130618093254.L1323>