support: Implement TEST_COMPARE_STRING
Commit Message
2018-11-05 Florian Weimer <fweimer@redhat.com>
Implement TEST_COMPARE_STRING.
* support/check.h (TEST_COMPARE_STRING): Define.
(support_test_compare_string): Declare.
* support/Makefile (libsupport-routines): Add
support_test_compare_string.
(tests): Add tst-test_compare_string.
* support/support_test_compare_string.c: New file.
* support/tst-test_compare_string.c: Likewise.
Comments
On Mon, 05 Nov 2018, Florian Weimer wrote:
>2018-11-05 Florian Weimer <fweimer@redhat.com>
>
> Implement TEST_COMPARE_STRING.
> * support/check.h (TEST_COMPARE_STRING): Define.
> (support_test_compare_string): Declare.
> * support/Makefile (libsupport-routines): Add
> support_test_compare_string.
> (tests): Add tst-test_compare_string.
> * support/support_test_compare_string.c: New file.
> * support/tst-test_compare_string.c: Likewise.
Looks good to me with a fix to some whitespace problems.
>+/* Compare the strings LEFT and RIGHT and report a test failure if
>+ they are different. Also report failure if one of the arguments is
>+ a null pointer and the other is not. The strings should be
>+ reasonably short because on mismatch, both are printed. */
>+#define TEST_COMPARE_STRING(left, right) \
>+ (support_test_compare_string (left, right, __FILE__, __LINE__, \
>+ #left, #right))
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Spaces instead of tabs.
>+void support_test_compare_string (const char *left, const char *right,
>+ const char *file, int line,
>+ const char *left_expr,
>+ const char *right_expr);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Likewise, here and throughout the patch.
>+ fputs (" ", stdout);
>+ for (size_t i = 0; i < length; ++i)
>+ printf (" %02X", blob[i]);
Thanks for adding this hex conversion. In the recently added test,
tst-ldbl-warn, it would have saved me some time in finding that a new line
wasn't being printed.
(I tried this patch on tst-ldbl-warn, by the way).
* Gabriel F. T. Gomes:
>>+/* Compare the strings LEFT and RIGHT and report a test failure if
>>+ they are different. Also report failure if one of the arguments is
>>+ a null pointer and the other is not. The strings should be
>>+ reasonably short because on mismatch, both are printed. */
>>+#define TEST_COMPARE_STRING(left, right) \
>>+ (support_test_compare_string (left, right, __FILE__, __LINE__, \
>>+ #left, #right))
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Spaces instead of tabs.
I wasn't aware that we have a policy to use tabs. Most of my changes
involving new files do not use tabs. 8-/
I personally find tabs problematic because they make it pretty much
impossible to review diffs for correct indentation, which is why I avoid
them in new files.
Thanks,
Florian
On Wed, 07 Nov 2018, Florian Weimer wrote:
>* Gabriel F. T. Gomes:
>
>>>+/* Compare the strings LEFT and RIGHT and report a test failure if
>>>+ they are different. Also report failure if one of the arguments is
>>>+ a null pointer and the other is not. The strings should be
>>>+ reasonably short because on mismatch, both are printed. */
>>>+#define TEST_COMPARE_STRING(left, right) \
>>>+ (support_test_compare_string (left, right, __FILE__, __LINE__, \
>>>+ #left, #right))
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> Spaces instead of tabs.
>
>I wasn't aware that we have a policy to use tabs. Most of my changes
>involving new files do not use tabs. 8-/
Oh, maybe we don't. Now I feel embarrassed that I complained about it so
many times.
>I personally find tabs problematic because they make it pretty much
>impossible to review diffs for correct indentation, which is why I avoid
>them in new files.
I agree.
On 07/11/18 11:36, Gabriel F. T. Gomes wrote:
> On Wed, 07 Nov 2018, Florian Weimer wrote:
>
>> * Gabriel F. T. Gomes:
>>
>>>> +/* Compare the strings LEFT and RIGHT and report a test failure if
>>>> + they are different. Also report failure if one of the arguments is
>>>> + a null pointer and the other is not. The strings should be
>>>> + reasonably short because on mismatch, both are printed. */
>>>> +#define TEST_COMPARE_STRING(left, right) \
>>>> + (support_test_compare_string (left, right, __FILE__, __LINE__, \
>>>> + #left, #right))
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> Spaces instead of tabs.
>>
>> I wasn't aware that we have a policy to use tabs. Most of my changes
>> involving new files do not use tabs. 8-/
>
> Oh, maybe we don't. Now I feel embarrassed that I complained about it so
> many times.
>
that's news to me,
why is there an awkward mix of space and tab
indentation everywhere in the code then?
>> I personally find tabs problematic because they make it pretty much
>> impossible to review diffs for correct indentation, which is why I avoid
>> them in new files.
>
> I agree.
>
* Szabolcs Nagy:
> On 07/11/18 11:36, Gabriel F. T. Gomes wrote:
>> On Wed, 07 Nov 2018, Florian Weimer wrote:
>>
>>> * Gabriel F. T. Gomes:
>>>
>>>>> +/* Compare the strings LEFT and RIGHT and report a test failure if
>>>>> + they are different. Also report failure if one of the arguments is
>>>>> + a null pointer and the other is not. The strings should be
>>>>> + reasonably short because on mismatch, both are printed. */
>>>>> +#define TEST_COMPARE_STRING(left, right) \
>>>>> + (support_test_compare_string (left, right, __FILE__, __LINE__, \
>>>>> + #left, #right))
>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> Spaces instead of tabs.
>>>
>>> I wasn't aware that we have a policy to use tabs. Most of my changes
>>> involving new files do not use tabs. 8-/
>>
>> Oh, maybe we don't. Now I feel embarrassed that I complained about it so
>> many times.
>>
>
> that's news to me,
>
> why is there an awkward mix of space and tab
> indentation everywhere in the code then?
It's the Emacs default. I don't know about vim.
Have we ever discussed changing it?
Thanks,
Florian
@@ -63,6 +63,7 @@ libsupport-routines = \
support_shared_allocate \
support_test_compare_blob \
support_test_compare_failure \
+ support_test_compare_string \
support_write_file_string \
support_test_main \
support_test_verify_impl \
@@ -198,6 +199,7 @@ tests = \
tst-support_record_failure \
tst-test_compare \
tst-test_compare_blob \
+ tst-test_compare_string \
tst-xreadlink \
ifeq ($(run-built-tests),yes)
@@ -163,6 +163,19 @@ void support_test_compare_blob (const void *left,
const char *right_exp,
const char *right_len_exp);
+/* Compare the strings LEFT and RIGHT and report a test failure if
+ they are different. Also report failure if one of the arguments is
+ a null pointer and the other is not. The strings should be
+ reasonably short because on mismatch, both are printed. */
+#define TEST_COMPARE_STRING(left, right) \
+ (support_test_compare_string (left, right, __FILE__, __LINE__, \
+ #left, #right))
+
+void support_test_compare_string (const char *left, const char *right,
+ const char *file, int line,
+ const char *left_expr,
+ const char *right_expr);
+
/* Internal function called by the test driver. */
int support_report_failure (int status)
__attribute__ ((weak, warn_unused_result));
new file mode 100644
@@ -0,0 +1,91 @@
+/* Check two strings for equality.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xmemstream.h>
+
+static void
+report_length (const char *what, const char *str, size_t length)
+{
+ if (str == NULL)
+ printf (" %s string: NULL\n", what);
+ else
+ printf (" %s string: %zu bytes\n", what, length);
+}
+
+static void
+report_string (const char *what, const unsigned char *blob,
+ size_t length, const char *expr)
+{
+ if (length > 0)
+ {
+ printf (" %s (evaluated from %s):\n", what, expr);
+ char *quoted = support_quote_blob (blob, length);
+ printf (" \"%s\"\n", quoted);
+ free (quoted);
+
+ fputs (" ", stdout);
+ for (size_t i = 0; i < length; ++i)
+ printf (" %02X", blob[i]);
+ putc ('\n', stdout);
+ }
+}
+
+static size_t
+string_length_or_zero (const char *str)
+{
+ if (str == NULL)
+ return 0;
+ else
+ return strlen (str);
+}
+
+void
+support_test_compare_string (const char *left, const char *right,
+ const char *file, int line,
+ const char *left_expr, const char *right_expr)
+{
+ /* Two null pointers are accepted. */
+ if (left == NULL && right == NULL)
+ return;
+
+ size_t left_length = string_length_or_zero (left);
+ size_t right_length = string_length_or_zero (right);
+
+ if (left_length != right_length || left == NULL || right == NULL
+ || memcmp (left, right, left_length) != 0)
+ {
+ support_record_failure ();
+ printf ("%s:%d: error: blob comparison failed\n", file, line);
+ if (left_length == right_length && right != NULL && left != NULL)
+ printf (" string length: %lu bytes\n", left_length);
+ else
+ {
+ report_length ("left", left, left_length);
+ report_length ("right", right, right_length);
+ }
+ report_string ("left", (const unsigned char *) left,
+ left_length, left_expr);
+ report_string ("right", (const unsigned char *) right,
+ right_length, right_expr);
+ }
+}
new file mode 100644
@@ -0,0 +1,107 @@
+/* Basic test for the TEST_COMPARE_STRING macro.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <string.h>
+#include <support/check.h>
+#include <support/capture_subprocess.h>
+
+static void
+subprocess (void *closure)
+{
+ /* These tests should fail. They were chosen to cover differences
+ in length (with the same contents), single-bit mismatches, and
+ mismatching null pointers. */
+ TEST_COMPARE_STRING ("", NULL); /* Line 29. */
+ TEST_COMPARE_STRING ("X", ""); /* Line 30. */
+ TEST_COMPARE_STRING (NULL, "X"); /* Line 31. */
+ TEST_COMPARE_STRING ("abcd", "abcD"); /* Line 32. */
+ TEST_COMPARE_STRING ("abcd", NULL); /* Line 33. */
+ TEST_COMPARE_STRING (NULL, "abcd"); /* Line 34. */
+}
+
+/* Same contents, different addresses. */
+char buffer_abc_1[] = "abc";
+char buffer_abc_2[] = "abc";
+
+static int
+do_test (void)
+{
+ /* This should succeed. Even if the pointers and array contents are
+ different, zero-length inputs are not different. */
+ TEST_COMPARE_STRING (NULL, NULL);
+ TEST_COMPARE_STRING ("", "");
+ TEST_COMPARE_STRING (buffer_abc_1, buffer_abc_2);
+ TEST_COMPARE_STRING (buffer_abc_1, "abc");
+
+ struct support_capture_subprocess proc = support_capture_subprocess
+ (&subprocess, NULL);
+
+ /* Discard the reported error. */
+ support_record_failure_reset ();
+
+ puts ("info: *** subprocess output starts ***");
+ fputs (proc.out.buffer, stdout);
+ puts ("info: *** subprocess output ends ***");
+
+ TEST_VERIFY
+ (strcmp (proc.out.buffer,
+"tst-test_compare_string.c:29: error: blob comparison failed\n"
+" left string: 0 bytes\n"
+" right string: NULL\n"
+"tst-test_compare_string.c:30: error: blob comparison failed\n"
+" left string: 1 bytes\n"
+" right string: 0 bytes\n"
+" left (evaluated from \"X\"):\n"
+" \"X\"\n"
+" 58\n"
+"tst-test_compare_string.c:31: error: blob comparison failed\n"
+" left string: NULL\n"
+" right string: 1 bytes\n"
+" right (evaluated from \"X\"):\n"
+" \"X\"\n"
+" 58\n"
+"tst-test_compare_string.c:32: error: blob comparison failed\n"
+" string length: 4 bytes\n"
+" left (evaluated from \"abcd\"):\n"
+" \"abcd\"\n"
+" 61 62 63 64\n"
+" right (evaluated from \"abcD\"):\n"
+" \"abcD\"\n"
+" 61 62 63 44\n"
+"tst-test_compare_string.c:33: error: blob comparison failed\n"
+" left string: 4 bytes\n"
+" right string: NULL\n"
+" left (evaluated from \"abcd\"):\n"
+" \"abcd\"\n"
+" 61 62 63 64\n"
+"tst-test_compare_string.c:34: error: blob comparison failed\n"
+" left string: NULL\n"
+" right string: 4 bytes\n"
+" right (evaluated from \"abcd\"):\n"
+" \"abcd\"\n"
+" 61 62 63 64\n"
+ ) == 0);
+
+ /* Check that there is no output on standard error. */
+ support_capture_subprocess_check (&proc, "TEST_COMPARE_STRING",
+ 0, sc_allow_stdout);
+
+ return 0;
+}
+
+#include <support/test-driver.c>