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

Re: [libvirt] [PATCH 08/11] util: Add util to pad string



On 06/07/2013 01:03 PM, Osier Yang wrote:
> The string can be padded either on the left (@from_right=false) or right
> (@from_right=true).
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virstring.c     | 38 ++++++++++++++++++++++++++++++++++++++
>  src/util/virstring.h     |  6 ++++++
>  tests/utiltest.c         | 28 ++++++++++++++++++++++++++++
>  4 files changed, 73 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ce39cc6..27fb0b5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1822,6 +1822,7 @@ virStringArrayHasString;
>  virStringFreeList;
>  virStringJoin;
>  virStringListLength;
> +virStringPad;
>  virStringSplit;
>  virStrncpy;
>  virStrndup;
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 1937f82..498daab 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -608,3 +608,41 @@ size_t virStringListLength(char **strings)
>  
>      return i;
>  }
> +

This needs a description, params, returns, etc. section.  To me padding
usually denotes adding something to the end of a string

I would like to understand the value of "length" (which I believe is a
size_t rather than a unsigned int, right?



I think most importantly - caller is expected to free the returned
buffer, right?


> +char *
> +virStringPad(const char *src,
> +             char padchar,
> +             unsigned int length,
> +             bool from_right)
> +{
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    int len;
> +    int i;


It seems you're trying to create a virBufferPrependChar() to go along
with the existing virBufferAddChar() or virBufferAddLit().

Essentially you're creating a string buffer of "length - len" filled
with 'padchar' and then prepending or appending it onto the existing
string.

I guess I'm somewhat surprised this hasn't already been done.

In any case, I would have expected something as follows (error
conditions aside):


if APPPEND  (eg, change 1234 into 12340000)
    virBufferAdd(&buf, src);
    for (i = len; i < length; i++)
        virBufferAddChar(&buf, padchar);
else  (PREPEND)
    for (i = len; i < length; i++)
        virBufferAddChar(&buf, padchar);
    virBufferAdd(&buf, src)


John
> +
> +    len = strlen(src);
> +
> +    if (len > length)
> +        return NULL;
> +
> +    if (!from_right) {
> +        for (i = 0; i < length - len; i++) {
> +            virBufferAsprintf(&buf, "%c", padchar);
> +        }
> +
> +        virBufferAsprintf(&buf, "%s", src);
> +    } else {
> +        virBufferAsprintf(&buf, "%s", src);
> +
> +        for (i = 0; i < length - len; i++) {
> +            virBufferAsprintf(&buf, "%c", padchar);
> +        }
> +    }
> +
> +    if (virBufferError(&buf)) {
> +        virBufferFreeAndReset(&buf);
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    return virBufferContentAndReset(&buf);
> +}
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index 34ffae1..5809167 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -166,4 +166,10 @@ int virStrndup(char **dest, const char *src, ssize_t n, bool report, int domcode
>  
>  size_t virStringListLength(char **strings);
>  
> +char *virStringPad(const char *src,
> +                   char padchar,
> +                   unsigned int length,
> +                   bool from_right)
> +    ATTRIBUTE_NONNULL(1);
> +
>  #endif /* __VIR_STRING_H__ */
> diff --git a/tests/utiltest.c b/tests/utiltest.c
> index 41fdd7e..d567ecc 100644
> --- a/tests/utiltest.c
> +++ b/tests/utiltest.c
> @@ -226,6 +226,33 @@ cleanup:
>  }
>  
>  static int
> +testStringPad(const void *data ATTRIBUTE_UNUSED)
> +{
> +    const char *str = "1ff2";
> +    char *lpadstr = NULL;
> +    char *rpadstr = NULL;
> +    int ret = -1;
> +
> +    if (!(lpadstr = virStringPad(str, '0', 7, false)))
> +        return -1;
> +
> +    if (STRNEQ(lpadstr, "0001ff2"))
> +        goto cleanup;
> +
> +    if (!(rpadstr = virStringPad(str, '0', 7, true)))
> +        goto cleanup;
> +
> +    if (STRNEQ(rpadstr, "1ff2000"))
> +        goto cleanup;
> +
> +    ret = 0;
> +cleanup:
> +    VIR_FREE(lpadstr);
> +    VIR_FREE(rpadstr);
> +    return ret;
> +}
> +
> +static int
>  mymain(void)
>  {
>      int result = 0;
> @@ -251,6 +278,7 @@ mymain(void)
>      DO_TEST(ParseVersionString);
>      DO_TEST(FindPCIDeviceByVPD);
>      DO_TEST(ParseStableScsiHostAddress);
> +    DO_TEST(StringPad);
>  
>  cleanup:
>      VIR_FREE(test_sysfs);
> 


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