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.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
---
 tests/testutils.c | 34 +++++++++++++---------------------
 tests/testutils.h | 10 +++++-----
 2 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index 01f3e8bb93..13bb9630df 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,36 +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 expect,
+                    unsigned long long actual)
 {
     VIR_AUTOFREE(char *) expectStr = NULL;
     VIR_AUTOFREE(char *) actualStr = NULL;
 
-    if (virAsprintf(&expectStr, "%llu", content) < 0)
+    if (virAsprintf(&expectStr, "%llu", expect) < 0)
         return -1;
 
-    if (virAsprintf(&actualStr, "%llu", src) < 0)
+    if (virAsprintf(&actualStr, "%llu", actual) < 0)
         return -1;
 
     return virTestCompareToString(expectStr, 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 *expect,
+                       const char *actual)
 {
-    if (STRNEQ_NULLABLE(strcontent, strsrc)) {
-        virTestDifference(stderr, strcontent, strsrc);
+    if (STRNEQ_NULLABLE(expect, actual)) {
+        virTestDifference(stderr, expect, actual);
         return -1;
     }
 
diff --git a/tests/testutils.h b/tests/testutils.h
index 658f9053ad..d0a259e2d4 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);
-int virTestCompareToString(const char *strcontent,
-                           const char *strsrc);
-int virTestCompareToULL(unsigned long long content,
-                        unsigned long long src);
+int virTestCompareToString(const char *expect,
+                           const char *actual);
+int virTestCompareToULL(unsigned long long expect,
+                        unsigned long long actual);
 
 unsigned int virTestGetDebug(void);
 unsigned int virTestGetVerbose(void);
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, 2019-02-20 at 14:20 +0100, Michal Privoznik wrote:
[...]
>  /*
> - * @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)

Agree with John that 'actual' being the first argument rather
than the second one here is pretty weird, but we can take care of
that in a follow-up commit. This change is already a massive
improvement over the status quo :)

[...]
> @@ -814,36 +814,28 @@ virTestCompareToFile(const char *strcontent,
>      return ret;
>  }
>  
> -/*
> - * @param content: Input content
> - * @param src: Source to compare @content against
> - */

Not sure why you removed the comments for this function and
virTestCompareToString() rather than just updating them as you've
done for the one above, but with the new names for the arguments
it's pretty much self-explanatory so I'm totally fine with it.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Feb 20, 2019 at 02:20:10PM +0100, 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.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>Reviewed-by: John Ferlan <jferlan@redhat.com>
>---
> tests/testutils.c | 34 +++++++++++++---------------------
> tests/testutils.h | 10 +++++-----
> 2 files changed, 18 insertions(+), 26 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list