From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v2 5/5] libxl, hvmloader: Don't relocate memory for MMIO hole Date: Thu, 20 Jun 2013 11:44:00 +0100 Message-ID: <51C2DCF0.2090902@eu.citrix.com> References: <1371573984-28514-1-git-send-email-george.dunlap@eu.citrix.com> <1371573984-28514-5-git-send-email-george.dunlap@eu.citrix.com> <51C2C9E0.6060006@eu.citrix.com> <51C2F1A502000078000DF4B7@nat28.tlf.novell.com> <51C2D77D.90103@eu.citrix.com> <20930.56179.543648.915942@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20930.56179.543648.915942@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Ian Campbell , Hanweidong , Stefano Stabellini , xen-devel@lists.xen.org, Stefano Stabellini , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 20/06/13 11:37, Ian Jackson wrote: > George Dunlap writes ("Re: [Xen-devel] [PATCH v2 5/5] libxl, hvmloader: Don't relocate memory for MMIO hole"): >> It may be pointless from a functionality perspective, but it's also >> harmless. It won't add a single byte to the compiled code, but the 6 >> characters will remind a developer reading the source that there is a >> cast being done here, just in case it should ever become important. Not >> super important, but I'd rather leave it in. > IMO this is a terrible reason for a cast. Casts are dangerous things > to have in code - they can override the compiler's normal > typechecking. They should be used only when actually necessary, and > code should normally be constructed so that they aren't. > >> Doing this would effectively hide the "default" value. This is bad >> because 1) it's not clear what the default is to someone just scanning >> the code, 2) it's hard to change. (Consider how you'd modify the above >> statement if you wanted to default to 0 instead.) > I agree with this complaint. Sorry, which complaint? Jan's complaint that we have an "unnecessary" if() statment, or my complaint that not having an if() statement is confusing? (That's the topic of this paragraph.) Or did you mean the complaint above, i.e., casting the result of strtoll()? I don't care terribly much about the casting; I'll change it rather than argue about it. :-) Does anyone has any opinions on getting rid of strtoll() altogether, and using direct comparisons, as Stefano suggests? -George