From owner-freebsd-bugs@FreeBSD.ORG Tue Feb 5 10:20:05 2013 Return-Path: Delivered-To: freebsd-bugs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id C1DAFC70; Tue, 5 Feb 2013 10:20:05 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx08.syd.optusnet.com.au (fallbackmx08.syd.optusnet.com.au [211.29.132.10]) by mx1.freebsd.org (Postfix) with ESMTP id 5EBA5B06; Tue, 5 Feb 2013 10:20:05 +0000 (UTC) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by fallbackmx08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r15AK4ha005052; Tue, 5 Feb 2013 21:20:04 +1100 Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r15AJrhA027918 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 5 Feb 2013 21:19:55 +1100 Date: Tue, 5 Feb 2013 21:19:53 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Gleb Smirnoff Subject: Re: kern/175759: Correct data types for fields of struct qm_trace{} from In-Reply-To: <20130205020931.GY91075@glebius.int.ru> Message-ID: <20130205210237.P1500@besplex.bde.org> References: <201302041420.r14EK4IP068305@freefall.freebsd.org> <20130205024617.X3554@besplex.bde.org> <20130205020931.GY91075@glebius.int.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=Uug7rJMB c=1 sm=1 a=Mvik2ojQbM4A:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=HXKBcPQmqRAA:10 a=Ck5FRmXBkoZy7QeH_vkA:9 a=CjuIK1q_8ugA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: freebsd-bugs@FreeBSD.org, Bruce Evans X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Feb 2013 10:20:05 -0000 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