[libvirt] [PATCH] testutils: Explicitly name virTestCompare*() arguments

[John Ferlan]

On 2/19/19 12:22 PM, Michal Privoznik wrote:
> Currently, some arguments are called strcontent and strsrc, or
> content and src or some other combination. This makes it
> impossible to see at the first glance what argument is supposed
> to represent 'expected' value and which one represents 'actual'
> value. Rename the arguments to make it obvious.
> 
> At the same time, rework virTestCompareToULL a bit so that local
> variables are named in the same fashion.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  tests/testutils.c | 51 +++++++++++++++++------------------------------
>  tests/testutils.h | 10 +++++-----
>  2 files changed, 23 insertions(+), 38 deletions(-)
> 
> diff --git a/tests/testutils.c b/tests/testutils.c
> index d2219ad21e..59c1d1fd6e 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -767,19 +767,19 @@ int virTestDifferenceBin(FILE *stream,
>  }
>  
>  /*
> - * @param strcontent: String input content
> - * @param filename: File to compare strcontent against
> + * @param actual: String input content
> + * @param filename: File to compare @actual against
>   *
> - * If @strcontent is NULL, it's treated as an empty string.
> + * If @actual is NULL, it's treated as an empty string.
>   */
>  int
> -virTestCompareToFile(const char *strcontent,
> +virTestCompareToFile(const char *actual,
>                       const char *filename)
>  {
>      int ret = -1;
>      char *filecontent = NULL;
>      char *fixedcontent = NULL;
> -    const char *cmpcontent = strcontent;
> +    const char *cmpcontent = actual;
>  
>      if (!cmpcontent)
>          cmpcontent = "";
> @@ -814,43 +814,28 @@ virTestCompareToFile(const char *strcontent,
>      return ret;
>  }
>  
> -/*
> - * @param content: Input content
> - * @param src: Source to compare @content against
> - */
>  int
> -virTestCompareToULL(unsigned long long content,
> -                    unsigned long long src)
> +virTestCompareToULL(unsigned long long expected,

Consistency with other virTestDifference* APIs would use "expect" rather
than "expected".  IDC which way you go though.

> +                    unsigned long long actual)
>  {
> -    char *strcontent = NULL;
> -    char *strsrc = NULL;
> -    int ret = -1;
> +    VIR_AUTOFREE(char *) expectedStr = NULL;
> +    VIR_AUTOFREE(char *) actualStr = NULL;

This conversion to VIR_AUTOFREE probably should be separate...

>  
> -    if (virAsprintf(&strcontent, "%llu", content) < 0)
> -        goto cleanup;
> +    if (virAsprintf(&expectedStr, "%llu", expected) < 0)
> +        return -1;
>  
> -    if (virAsprintf(&strsrc, "%llu", src) < 0)
> -        goto cleanup;
> +    if (virAsprintf(&actualStr, "%llu", actual) < 0)
> +        return -1;
>  
> -    ret = virTestCompareToString(strcontent, strsrc);
> -
> - cleanup:
> -    VIR_FREE(strcontent);
> -    VIR_FREE(strsrc);
> -
> -    return ret;
> +    return virTestCompareToString(expectedStr, actualStr);
>  }
>  
> -/*
> - * @param strcontent: String input content
> - * @param strsrc: String source to compare strcontent against
> - */
>  int
> -virTestCompareToString(const char *strcontent,
> -                       const char *strsrc)
> +virTestCompareToString(const char *expected,

similar "expect"

> +                       const char *actual)
>  {
> -    if (STRNEQ_NULLABLE(strcontent, strsrc)) {
> -        virTestDifference(stderr, strcontent, strsrc);
> +    if (STRNEQ_NULLABLE(expected, actual)) {
> +        virTestDifference(stderr, expected, actual);
>          return -1;
>      }
>  
> diff --git a/tests/testutils.h b/tests/testutils.h
> index 658f9053ad..1ed9f0b6d3 100644
> --- a/tests/testutils.h
> +++ b/tests/testutils.h
> @@ -76,12 +76,12 @@ int virTestDifferenceBin(FILE *stream,
>                           const char *expect,
>                           const char *actual,
>                           size_t length);
> -int virTestCompareToFile(const char *strcontent,
> +int virTestCompareToFile(const char *actual,
>                           const char *filename);

Is it just me that's bothered by just 1 of these not being like the
others with expect as param1 and actual as param2. Different problem though.

> -int virTestCompareToString(const char *strcontent,
> -                           const char *strsrc);
> -int virTestCompareToULL(unsigned long long content,
> -                        unsigned long long src);
> +int virTestCompareToString(const char *expected,
> +                           const char *actual);
> +int virTestCompareToULL(unsigned long long expected,
> +                        unsigned long long actual);

If you change the expected to expect don't forget these.

Assuming extraction (sigh) of the VIR_AUTOFREE,

Reviewed-by: John Ferlan <jferlan at redhat.com>

John


>  
>  unsigned int virTestGetDebug(void);
>  unsigned int virTestGetVerbose(void);
>