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@xxxxxxxxxx> >> --- >> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list