From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MggRx-0005BL-NI for qemu-devel@nongnu.org; Thu, 27 Aug 2009 10:59:41 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MggRt-00059J-0q for qemu-devel@nongnu.org; Thu, 27 Aug 2009 10:59:41 -0400 Received: from [199.232.76.173] (port=56057 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MggRs-000591-Ny for qemu-devel@nongnu.org; Thu, 27 Aug 2009 10:59:36 -0400 Received: from mail2.shareable.org ([80.68.89.115]:45182) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MggRs-0008G4-0d for qemu-devel@nongnu.org; Thu, 27 Aug 2009 10:59:36 -0400 Received: from jamie by mail2.shareable.org with local (Exim 4.63) (envelope-from ) id 1MggRq-0000Gm-Dp for qemu-devel@nongnu.org; Thu, 27 Aug 2009 15:59:34 +0100 Date: Thu, 27 Aug 2009 15:59:34 +0100 From: Jamie Lokier Subject: Re: [Qemu-devel] [PATCH] Remove typedef for bool from eepro100.c Message-ID: <20090827145934.GD31453@shareable.org> References: <1251376284-22426-1-git-send-email-amit.shah@redhat.com> <20090827124546.GA5912@1und1.de> <761ea48b0908270700i32fbaab9kb23d3cd2bfd09ebe@mail.gmail.com> <20090827144214.GA3803@1und1.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20090827144214.GA3803@1und1.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Reimar Döffinger wrote: > On Thu, Aug 27, 2009 at 04:00:17PM +0200, Laurent Desnogues wrote: > > On Thu, Aug 27, 2009 at 2:45 PM, Reimar > > Döffinger wrote: > > > 1) stdbool bool is probably 4 bytes, not just 1 like char > > > > It's one byte on my gcc 4.4.0. > > > > > I suggest to just get rid of bool in this file, it is only used in 5 > > > places, i.e. change > > >>        bool bit_el = ((command & 0x8000) != 0); > > > to > > >>        int bit_el = command & 0x8000; > > > > This is dangerous if you start using bit_el in integer expressions > > by accident (for instance using & or |). > > Programming errors are dangerous in general. I don't see much of a > point of cluttering the code with not really effective ways to hide > their effects (unless you wanted to suggest using "bool bit_el = > command & 0x8000;", I see that bool is already used in some places > in qemu and performance doesn't matter here so it indeed shouldn't > be a problem). I've seen the "int flag = (x & FLAG); if (flag & otherflag)" bug enough times to consider it one of those subtle things that programmers don't notice easily. A variation is "long flag = (x & FLAG); ... function(flag)" where "function" takes an int argument and FLAG doesn't fit. Or just "int flag = (x & FLAG)". My approach is to use "!= 0" if I think the variable really is boolean, and only keep the original masked bit pattern if it's clear in context that the variable is supposed to contain a bit pattern. -- Jamie