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

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



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]
>
> * src/conf/storage_encryption_conf.c
> (virStorageEncryptionSecretFormat, virStorageEncryptionFormat):
> Use correct type.
> * src/conf/storage_encryption_conf.h (virStorageEncryptionFormat):
> Likewise.
> ---
>
>> > +    virBufferVSprintf(buf, "%*s</encryption>\n", (int) indent, "");
>> >
>> What's the need for the cast if the function parameter is changed to
>> int instead of unsigned int?  The two callers pass in hardcoded
>> values, so I'd think just changing the param would silence the
>> warning, wouldn't it?
>
> Sure - it's just one more file to touch.  Change from v1: update
> the .h file to use fewer casts.
>
>  src/conf/storage_encryption_conf.c |    9 +++++----
>  src/conf/storage_encryption_conf.h |    4 ++--
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c
> index 7a64050..7bbdbc1 100644
> --- a/src/conf/storage_encryption_conf.c
> +++ b/src/conf/storage_encryption_conf.c
> @@ -1,7 +1,7 @@
>  /*
>   * storage_encryption_conf.c: volume encryption information
>   *
> - * Copyright (C) 2009 Red Hat, Inc.
> + * Copyright (C) 2009-2010 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -216,7 +216,7 @@ virStorageEncryptionParseNode(xmlDocPtr xml, xmlNodePtr root)
>  static int
>  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.

Of course, avoiding casts is good, too, but IMHO, not if
it makes us obfuscate (even ever so slightly) the types we use.

>  {
>      const char *type;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
> @@ -237,7 +237,7 @@ virStorageEncryptionSecretFormat(virBufferPtr buf,
>  int
>  virStorageEncryptionFormat(virBufferPtr buf,
>                             virStorageEncryptionPtr enc,
> -                           unsigned int indent)
> +                           int indent)


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