On 05/10/2010 01:02 PM, Jim Meyering wrote:
Eric Blake wrote:
> For printf("%*s",foo,bar), clang complains if foo is not int:
>
> warning: field width should have type 'int', but argument has
> type 'unsigned int' [-Wformat]
>
> virStorageEncryptionSecretFormat(virBufferPtr buf,
> virStorageEncryptionSecretPtr secret,
> - unsigned int indent)
> + int indent)
"unsigned int" sounds like the right type to me, since
"indent" never goes negative. And using the unsigned type
seems to be in line with policy in HACKING:
If a variable is counting something, be sure to declare it with an
unsigned type.
But printf("%*s", indent, string) would indeed behave differently if
indent goes negative (if signed) or larger than INT_MAX (if unsigned),
so clang's warning is realistic, and worth silencing in our quest to get
a clean clang build. About the only other thing I could think of to do
that would avoid confusion and also to avoid casts is to keep the public
interface with unsigned int, then add an intermediate helper variable:
int real_indent = indent;
and use real_indent in the printf call, but that seems like overkill.
Of course, avoiding casts is good, too, but IMHO, not if
it makes us obfuscate (even ever so slightly) the types we use.
Well, the real point of this patch was to silence a compiler warning
(not that we'd ever planning on passing an indent > 2G). Given Dave's
ACK, I went ahead and applied v2 as proposed, even if it does slightly
obfuscate the usage.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org