Re: [PATCH RFC] xl_cmdimpl.c: Fix printf usage

[George Dunlap <george.dunlap@citrix.com>]

On 12/10/16 19:52, Ronald Rojas wrote:
> Change instances of printf, fprintf, and LOG where the specifier
> used is '%d' to be '%u' for domid.
> 
> Signed-off-by: Ronald Rojas <ronladred@gmail.com>

Code looks good, thanks!

A couple of minor adjustments to the patch itself:

First, the traditional "tag" for this would probably be "tools/xl"
instead of the name of the file.  Secondly one-line description needs to
be a bit more specific, so people skimming through have an idea whether
they need to look further at what the patch does or not.  I think
something like "Use %u for uint32_t domids" would give the general idea.

Then in your full changelog, you want to say what the *intention* of the
patch is first, and *then* what the patch does.  That is, describe the
status quo, why that's a problem, and what you want to achieve.  You can
leave some of it out if it's obvious, but what you have now is a bit too
much left out.

Something like this:

domid is normally represented by uint32_t, but many format strings in
xl_cmdimpl.c use %d when printing it, which is signed.  Use %u instead.

With those changes you can re-send without the RFC.

Thanks,
 -George


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