[libvirt] [PATCH v2 1/5] introduce virSnprintf helper

Eric Blake eblake at redhat.com
Fri Sep 12 18:59:35 UTC 2014


On 09/12/2014 12:47 PM, John Ferlan wrote:
> 
> 
> On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> ---
>>  src/libvirt_private.syms |  1 +
>>  src/util/virstring.c     | 25 +++++++++++++++++++++++++
>>  src/util/virstring.h     | 20 ++++++++++++++++++++
>>  3 files changed, 46 insertions(+)
>>
> 
> Not sure why this is necessary - I do see calls to 'snprintf' from
> within existing libvirt code.  The usage you have in patch 5 could
> easily use snprintf. There is no need for variable argument list.
> 
> I'm not against it, but if it's created shouldn't other snprintf's use it?

I agree that if we introduce this, we should add a syntax check that
forbids raw use of snprintf, and fix all existing callers to start using
it.  It does have the nice aspect of doing automatic error reporting if
the buffer is too small, so I'm not opposed to adding it, but it would
need another version.  I'm also okay if you just directly use snprintf
(seeing as how we already have existing code that does that) without
trying to add this wrapper.

>> +/**
>> + * virSnprintf:
>> + * @strp: variable to hold result (char*)
>> + * @fmt: printf format
>> + *
>> + * Like glibc's_snprintf but report error if the src string is longer
>> + * then dst string.

Wouldn't that really be "report error if the resulting output is too
long for the dst string"?

>> + *
>> + * Returns -1 on failure, number of bytes printed on success.
>> + */
>> +
>> +# define virSnprintf(strp, ...) \
>> +    virSnprintfInternal(VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__, \
>> +                        strp, ARRAY_CARDINALITY(strp), __VA_ARGS__)

Hmm, this doesn't match your documentation.  ARRAY_CARDINALITY only
works on char[], not on char* (that is, the compiler has to know how
long the array is, not just that it is a pointer).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140912/d1a905ec/attachment-0001.sig>


More information about the libvir-list mailing list