Date: Tue, 5 Feb 2013 03:23:44 +1100 (EST)
From: Bruce Evans <brde@optusnet.com.au>
To: Gleb Smirnoff <glebius@freebsd.org>
Cc: freebsd-bugs@freebsd.org
Subject: Re: kern/175759: Correct data types for fields of struct qm_trace{} from <sys/queue.h>
Message-ID: <20130205024617.X3554@besplex.bde.org>
In-Reply-To: <201302041420.r14EK4IP068305@freefall.freebsd.org>
References: <201302041420.r14EK4IP068305@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 4 Feb 2013, Gleb Smirnoff wrote:
> any additional comments for the attached patch. Is it ok from your
> viewpoint?
> Index: queue.h
> ===================================================================
> --- queue.h (revision 245741)
> +++ queue.h (working copy)
> @@ -105,13 +105,14 @@
> #ifdef QUEUE_MACRO_DEBUG
> /* Store the last 2 places the queue element or head was altered */
> struct qm_trace {
> - char * lastfile;
> - int lastline;
> - char * prevfile;
> - int prevline;
> + const char * lastfile;
> + unsigned long lastline;
> + const char * prevfile;
> + unsigned long prevline;
> };
Unsigned long is unnecessarily large. It wastes space on 64-bit
arches. The change doesn't change the wastage, because space was
already wasted on 64-bit arches by mispacking the struct (with
unnamed padding after the ints). It changes the API unnecessarily
by changing signed variables to unsigned. Sign variables are
easier to use, and changing to unsigned ones risks sign extension
bugs.
According to your quote of the C standard, int32_t is enough. (I
couldn't find anything directly about either the type or limit of
__LINE__ in the n869.txt draft of C99, but #line is limited to 2**31-1.
n1124.pdf says much the same, except it says that __LINE__ is an integer
constant where n869.txt says that __LINE__ is a decimal constant. Both
of these seem to be wrong -- "decimal constants" include floating point
ones, and "integer constants" include octal and hex ones.)
The change preserves many style bugs:
- no prefix in member names
- member names not sorted inversely on size. This gives the space
wastage.
- member names not indented
- space between '*' and member names.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130205024617.X3554>