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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
John
>
> unsigned int virTestGetDebug(void);
> unsigned int virTestGetVerbose(void);
>
