[PATCH v6] string: Adds tests for test-strncasecmp and test-strncpy
Paul A. Clarke
pc@us.ibm.com
Thu Jul 9 15:27:37 GMT 2020
On Wed, Jul 08, 2020 at 05:20:09PM -0300, Raphael Moreira Zinsly wrote:
> Changes since v5:
> string/test-strncpy.c:
> - Adjusted s1 and s2 placements.
> - Used the same pattern as test-strncasecmp.
>
> --- >8 ---
>
> Adds tests to check if strings placed at page bondaries are
> handled correctly by strncasecmp and strncpy similar to tests
> for strncmp and strnlen.
> ---
> string/test-strncasecmp.c | 44 +++++++++++++++++++++++++++++++++++++++
> string/test-strncpy.c | 36 ++++++++++++++++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6a9c27beae..628135b962 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -137,6 +137,49 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
> do_one_test (impl, s1, s2, n, exp_result);
> }
>
> +static void
> +do_page_tests (void)
> +{
> + char *s1, *s2;
> + int exp_result;
> + /* Assumes up to 512-bit wide read/stores. */
Consider more explanation here.
> + const size_t maxoffset = 64;
> +
> + s1 = (char *) buf1 + (BUF1PAGES) * page_size - maxoffset;
You shouldn't need the parens around BUF1PAGES.
> + memset (s1, 'a', maxoffset - 1);
> + s1[maxoffset - 1] = '\0';
> +
> + s2 = (char *) buf2 + page_size - maxoffset;
> + memset (s2, 'a', maxoffset - 1);
> + s2[maxoffset - 1] = '\0';
> +
> + /* At this point s1 and s2 points to distinct memory regions containing
nit (English): s/points/point/
> + "aa..." with size of 63 plus '\0'. Also, both strings are bounded to a
> + page with write/read access and the next page is protected with PROT_NONE
nit, and totally subjective, but since I'm here, it's more common to use "read/write".
> + (meaning that any access outside of the page regions will trigger an
> + invalid memory access).
> +
> + The loop checks for all possible offset up to maxoffset for both
nit: s/offset/offsets/
> + inputs with a size larger than the string (so memory access outside
> + the expected memory regions might trigger invalid access). */
> +
> + for (size_t off1 = 0; off1 < maxoffset; off1++)
> + {
> + for (size_t off2 = 0; off2 < maxoffset; off2++)
> + {
> + exp_result = off1 == off2
nit, subjective: consider parens around "off1 == off2" for readability.
> + ? 0
> + : off1 < off2
> + ? 'a'
> + : -'a';
> +
> + FOR_EACH_IMPL (impl, 0)
> + check_result (impl, s1 + off1, s2 + off2, maxoffset + 1,
> + exp_result);
> + }
> + }
> +}
> +
> static void
> do_random_tests (void)
> {
> @@ -334,6 +377,7 @@ test_locale (const char *locale)
> }
>
> do_random_tests ();
> + do_page_tests ();
> }
>
> int
> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
> index c978753ad8..c8d4ad58a6 100644
> --- a/string/test-strncpy.c
> +++ b/string/test-strncpy.c
> @@ -155,6 +155,41 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
> do_one_test (impl, s2, s1, len, n);
> }
>
> +static void
> +do_page_tests (void)
> +{
> + CHAR *s1, *s2;
> + /* Assumes up to 512-bit wide read/stores. */
> + const size_t maxoffset = 64;
> +
> + /* Put s1 at the edge of buf1's last page. */
> + s1 = (CHAR *) buf1 + (BUF1PAGES) * (page_size / sizeof(CHAR)) - maxoffset;
Can remove parens around BUF1PAGES and "page_size / sizeof(CHAR)".
> + /* Put s2 almost at the edge of buf2, it needs room to put a string with
> + size of maxoffset + 1 at s2 + maxoffset. */
> + s2 = (CHAR *) buf2 + (page_size / sizeof(CHAR)) - (maxoffset + 1) * 2;
Could this be made more similar to the above computation for s1?
Why the "* 2"? I guess there is no definition for BUF2PAGES. Ugh.
I propose adding one, to avoid magic numbers.
> +
> + MEMSET (s1, 'a', maxoffset - 1);
> + s1[maxoffset - 1] = '\0';
> +
> + /* Both strings are bounded to a page with write/read access and the next
nit, and totally subjective, but since I'm here, it's more common to use "read/write".
> + page is protected with PROT_NONE (meaning that any access outside of the
> + page regions will trigger an invalid memory access).
> +
> + The loop copies the string s1 for all possible offset up to maxoffset
nit: s/offset/offsets/
> + for both inputs with a size larger than s1 (so memory access outside the
> + expected memory regions might trigger invalid access). */
> +
> + for (size_t off1 = 0; off1 < maxoffset; off1++)
> + {
> + for (size_t off2 = 0; off2 < maxoffset; off2++)
> + {
> + FOR_EACH_IMPL (impl, 0)
> + do_one_test (impl, (s2 + off2), (s1 + off1), maxoffset - off1 - 1,
> + maxoffset + 1);
> + }
> + }
> +}
> +
> static void
> do_random_tests (void)
> {
> @@ -317,6 +352,7 @@ test_main (void)
> }
>
> do_random_tests ();
> + do_page_tests ();
> return ret;
> }
LGTM with the above addressed.
PC
More information about the Libc-alpha
mailing list