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

Re: [libvirt] [PATCHv2] storage_encryption: silence clang warning



On Mon, May 10, 2010 at 02:12:29PM -0600, Eric Blake wrote:
> 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.

It's valid to pass a negative value, so int is the correct type.  My
bad for using unsigned int in the first place.

>From man printf:

A negative field width is taken as a '-' flag followed by a positive
field width.

Dave


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