[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 20/06/13 11:12, Jan Beulich wrote:
On 20.06.13 at 11:22, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
On 19/06/13 18:18, Stefano Stabellini wrote:
On Tue, 18 Jun 2013, George Dunlap wrote:
+    s = xenstore_read(HVM_XS_ALLOW_MEMORY_RELOCATE, NULL);
+    if ( s )
+        allow_memory_relocate = (bool)strtoll(s, NULL, 0);
+    printf("Relocating guest memory for lowmem MMIO space %s\n",
+           allow_memory_relocate?"enabled":"disabled");
It doesn't take a strtoll to parse a boolean.
As discussed in v1, strtoll is the only "XtoY" function available in
hvmloader. :-)  The only other option would be to explicitly compare for
"1" or "0" (or do some half-baked *s-'0' thing).

This does make me think though -- what is the semantics of casting to a
bool?  Is it !!, or will it essentially clip off the high bits?  (e.g.,
would "2" become "1", or "0"?)
If bool is a typedef or #define of _Bool, and _Bool is a complier
supplied type, then the cast will do the right thing. But doing the
assignment without the cast would too, i.e. the cast is pointless
(as I think IanJ had already pointed out).

Thanks for the info.

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.

However, if we want to be on the safe side and also make the
code work with a compiler that doesn't have a built-in _Bool, I'd

     allow_memory_relocate = !s || strtoll(s, NULL, 0);

would be the better statement (without any if() surrounding it,
and without the variable declaration having an initializer.

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.)


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.