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

Re: [libvirt] [PATCH 1/2] virStrndup: Accept negative values as string length



On 05/24/2013 03:53 AM, Michal Privoznik wrote:
> It may shorten the code a bit as the following pattern:
> 
>   VIR_STRNDUP(dst, src, cond ? n : strlen(src))
> 
> is used on several places among our code. However, we can
> move the strlen into virStrndup and thus write just:
> 
>   VIR_STRNDUP(dst, src, cond ? n : -1)
> ---
>  src/util/virstring.c  |  7 ++++++-
>  src/util/virstring.h  | 11 ++++++++---
>  tests/virstringtest.c | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 50 insertions(+), 4 deletions(-)
> 

> @@ -132,7 +132,9 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
>   * @n: the maximum number of bytes to copy
>   *
>   * Duplicate @src string and store it into @dst. If @src is longer than @n,
> - * only @n bytes are copied and terminating null byte '\0' is added.
> + * only @n bytes are copied and terminating null byte '\0' is added. If @n
> + * is a negative number, then the whole @src string is copied. That is

s/That is/That is,/

> + * VIR_STRDUP(dst, src) and VIR_STRNDUP(dst, src, -1) are equal.
>   *
>   * This macro is safe to use on arguments with side effects.
>   *
> @@ -150,7 +152,10 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
>   * @n: the maximum number of bytes to copy
>   *
>   * Duplicate @src string and store it into @dst. If @src is longer than @n,
> - * only @n bytes are copied and terminating null byte '\0' is added.
> + * only @n bytes are copied and terminating null byte '\0' is added. If @n
> + * is a negative number, then the whole @src string is copied. That is

and again.

> +static int
> +testStrndupNegative(const void *opaque ATTRIBUTE_UNUSED)
> +{
> +    int ret = -1;
> +    char *dst;
> +    const char *src = "Hello world";
> +    int value;
> +
> +    if ((value = VIR_STRNDUP(dst, src, 5)) != 1) {
> +        fprintf(stderr, "unexpected virStrndup result %d, expected 1\n", value);
> +        goto cleanup;
> +    }
> +
> +    if (STRNEQ_NULLABLE(dst, "Hello")) {
> +        fprintf(stderr, "unexpected content '%s'", dst);
> +        goto cleanup;
> +    }
> +
> +    if ((value = VIR_STRNDUP(dst, src, -1)) != 1) {

Memory leak.  VIR_FREE(dst) before doing another dup into it.

ACK with those fixes.

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

Attachment: signature.asc
Description: OpenPGP digital signature


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