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

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




On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina 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?

Hopefully someone else has an opinion on this...

John


> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fdf4548..80d8e7d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1956,6 +1956,7 @@ virAsprintfInternal;
>  virSkipSpaces;
>  virSkipSpacesAndBackslash;
>  virSkipSpacesBackwards;
> +virSnprintfInternal;
>  virStrcpy;
>  virStrdup;
>  virStringArrayHasString;
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index b14f785..2a657b3 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -483,6 +483,31 @@ virAsprintfInternal(bool report,
>      return ret;
>  }
>  
> +int
> +virSnprintfInternal(int domcode,
> +                    const char *filename,
> +                    const char *funcname,
> +                    size_t linenr,
> +                    char *strp,
> +                    size_t strLen,
> +                    const char *fmt, ...)
> +{
> +    va_list ap;
> +    int ret;
> +
> +    va_start(ap, fmt);
> +    ret = vsnprintf(strp, strLen, fmt, ap);
> +
> +    if (ret >= strLen)
> +        virReportErrorHelper(domcode, VIR_ERR_INTERNAL_ERROR,
> +                             filename, funcname, linenr,
> +                             _("Size of src string '%d' is greater than available size of dst string '%zu'"),
> +                             ret, strLen);
> +    va_end(ap);
> +    return ret;
> +
> +}
> +
>  /**
>   * virStrncpy
>   *
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index 267fbd0..c526c12 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -129,6 +129,11 @@ int virVasprintfInternal(bool report, int domcode, const char *filename,
>                           const char *fmt, va_list list)
>      ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7) ATTRIBUTE_FMT_PRINTF(7, 0)
>      ATTRIBUTE_RETURN_CHECK;
> +int virSnprintfInternal(int domcode, const char *filename, const char *funcname,
> +                        size_t linenr, char *strp, size_t strLen,
> +                        const char *fmt, ...)
> +    ATTRIBUTE_NONNULL(5) ATTRIBUTE_FMT_PRINTF(7, 8)
> +    ATTRIBUTE_RETURN_CHECK;
>  
>  /**
>   * VIR_STRDUP:
> @@ -252,6 +257,21 @@ size_t virStringListLength(char **strings);
>      virAsprintfInternal(false, 0, NULL, NULL, 0, \
>                          strp, __VA_ARGS__)
>  
> +/**
> + * 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.
> + *
> + * 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__)
> +
>  int virStringSortCompare(const void *a, const void *b);
>  int virStringSortRevCompare(const void *a, const void *b);
>  
> 


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