On Tue, 5 Feb 2013, Gleb Smirnoff wrote:
On Tue, Feb 05, 2013 at 03:23:44AM +1100, Bruce Evans wrote: B> On Mon, 4 Feb 2013, Gleb Smirnoff wrote: B> B> > any additional comments for the attached patch. Is it ok from your B> > viewpoint? B> B> > Index: queue.h B> > =================================================================== B> > --- queue.h (revision 245741) B> > +++ queue.h (working copy) B> > @@ -105,13 +105,14 @@ B> > #ifdef QUEUE_MACRO_DEBUG B> > /* Store the last 2 places the queue element or head was altered */ B> > struct qm_trace { B> > - char * lastfile; B> > - int lastline; B> > - char * prevfile; B> > - int prevline; B> > + const char * lastfile; B> > + unsigned long lastline; B> > + const char * prevfile; B> > + unsigned long prevline; B> > }; B> B> Unsigned long is unnecessarily large. It wastes space on 64-bit B> arches. The change doesn't change the wastage, because space was B> already wasted on 64-bit arches by mispacking the struct (with B> unnamed padding after the ints). It changes the API unnecessarily B> by changing signed variables to unsigned. Sign variables are B> easier to use, and changing to unsigned ones risks sign extension B> bugs. B> B> According to your quote of the C standard, int32_t is enough. (I B> couldn't find anything directly about either the type or limit of B> __LINE__ in the n869.txt draft of C99, but #line is limited to 2**31-1. B> n1124.pdf says much the same, except it says that __LINE__ is an integer B> constant where n869.txt says that __LINE__ is a decimal constant. Both B> of these seem to be wrong -- "decimal constants" include floating point B> ones, and "integer constants" include octal and hex ones.)As Andrey pointed out, int may be smaller than 2**31-1, that's why longs are used.
Using int would only be a style bug, since FreeBSD has thousands if not millions of other assumptions that ints are precisely 32 bits. Anyway, int32_t is large enough to hold 2**31-1.
I know that you prefer signed variables since they are easier to use, but I prefer to explictily use unsigned in places where value can not go below zero by its definition.
I used to prefer the latter, but know better now :-). __LINE__ constant literals probably have type int or long, so it is inconsistent to store them as unsigned. But I can't think of any useful expression where the behaviour would be different due to not being unsigned -- the expression (p1->lastline - p2->lastline) might be useful (if unsigned is not used to break it), but there is no similar expression with 2 __LINE__ constants. Bruce Bruce _______________________________________________ freebsd-bugs@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
