Re: [Xen-devel] [PATCH v2 5/5] libxl, hvmloader: Don't relocate memory for MMIO hole

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

A straight assignment to _Bool is guaranteed to DTRT.  If that's too
opaque because the type of the variable is hidden elsewhere, you can
use the boolean canonicalisation operator, !!.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel