string/test-strncmp.c: Add comments within do_random_tests()

Message ID 38bf371c-c7ae-8a5b-24f0-08d8ffce270f@us.ibm.com
State New, archived
Headers

Commit Message

Paul A. Clarke March 28, 2017, 8:06 p.m. UTC
  I was about to throw this out after abandoning my attempts to "fix"
string/test-strncmp.c (which didn't need fixing).

However, since I added some comments which I found useful in
navigating the code, I thought I should offer them for inclusion.

-- 8< --

I added comments and rearranged some code to clarify the workings
of do_random_tests() in the hope that someone new to the code will
find it much more comprehensible than I did.

2017-03-28  Paul A. Clarke  <pc@us.ibm.com>

	* string/test-strncmp.c (do_random_tests): Add comments and rearrange
	some code for readability.
---
  string/test-strncmp.c | 40 +++++++++++++++++++++++++++++++++++++---
  1 file changed, 37 insertions(+), 3 deletions(-)
  

Comments

Wainer dos Santos Moschetta March 30, 2017, 3:48 p.m. UTC | #1
LGTM.


On 03/28/2017 05:06 PM, Paul Clarke wrote:
> I was about to throw this out after abandoning my attempts to "fix"
> string/test-strncmp.c (which didn't need fixing).
>
> However, since I added some comments which I found useful in
> navigating the code, I thought I should offer them for inclusion.
>
> -- 8< --
>
> I added comments and rearranged some code to clarify the workings
> of do_random_tests() in the hope that someone new to the code will
> find it much more comprehensible than I did.
>
> 2017-03-28  Paul A. Clarke  <pc@us.ibm.com>
>
>     * string/test-strncmp.c (do_random_tests): Add comments and rearrange
>     some code for readability.
> ---
>  string/test-strncmp.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/string/test-strncmp.c b/string/test-strncmp.c
> index fe3c4e3..dabf1f1 100644
> --- a/string/test-strncmp.c
> +++ b/string/test-strncmp.c
> @@ -270,33 +270,58 @@ do_random_tests (void)
>    size_t i, j, n, align1, align2, pos, len1, len2, size;
>    int result;
>    long r;
> +  /* Get pointers near the end of a "page".  */
>    UCHAR *p1 = (UCHAR *) (buf1 + page_size - 512 * CHARBYTES);
>    UCHAR *p2 = (UCHAR *) (buf2 + page_size - 512 * CHARBYTES);
>  
>    for (n = 0; n < ITERATIONS; n++)
>      {
> +      /* Start index for first string within p1.  */
>        align1 = random () & 31;
> +
> +      /* Start index for second string within p2.  */
>        if (random () & 1)
>      align2 = random () & 31;
>        else
>      align2 = align1 + (random () & 24);
> -      pos = random () & 511;
> -      size = random () & 511;
> +
> +      /* Compute larger offset into buffers.  */
>        j = align1 > align2 ? align1 : align2;
> -      if (pos + j >= 511)
> +
> +      /* Position of difference between strings.  */
> +      pos = random () & 511;
> +
> +      /* Ensure pos within range for strings.  */
> +      if (j + pos >= 511)
>      pos = 510 - j - (random () & 7);
> +
> +      /* Maximum size of operation.  */
> +      size = random () & 511;
> +
> +      /* Note: size does not need to be capped to the page boundary, as either
> +     - size >= pos, so the null ('\0') at 'pos' will terminate strncmp, or
> +     - size < pos, so 'size' will terminate strncmp.  */
> +
> +      /* Actual length of 1st string.  */
>        len1 = random () & 511;
>        if (pos >= len1 && (random () & 1))
>      len1 = pos + (random () & 7);
>        if (len1 + j >= 512)
>      len1 = 511 - j - (random () & 7);
> +
> +      /* Actual length of 2nd string.  */
> +      /* Note: len2 >= len1.  */
>        if (pos >= len1)
>      len2 = len1;
>        else
>      len2 = len1 + (len1 != 511 - j ? random () % (511 - j - len1) : 0);
> +
> +      /* Compute max length of strings, plus margin of dword.  */
>        j = (pos > len2 ? pos : len2) + align1 + 64;
>        if (j > 512)
>      j = 512;
> +
> +      /* Fill p1 with random data.  */
>        for (i = 0; i < j; ++i)
>      {
>        p1[i] = random () & 255;
> @@ -307,6 +332,8 @@ do_random_tests (void)
>          p1[i] = 1 + (random () & 127);
>          }
>      }
> +
> +      /* Fill p2 with random data.  */
>        for (i = 0; i < j; ++i)
>      {
>        p2[i] = random () & 255;
> @@ -319,11 +346,15 @@ do_random_tests (void)
>      }
>  
>        result = 0;
> +
> +      /* Make strings the same, up to position pos.  */
>        MEMCPY (p2 + align2, p1 + align1, pos);
> +
>        if (pos < len1)
>      {
>        if (p2[align2 + pos] == p1[align1 + pos])
>          {
> +          /* Insert difference.  */
>            p2[align2 + pos] = random () & 255;
>            if (p2[align2 + pos] == p1[align1 + pos])
>          p2[align2 + pos] = p1[align1 + pos] + 3 + (random () & 127);
> @@ -331,12 +362,15 @@ do_random_tests (void)
>  
>        if (pos < size)
>          {
> +          /* Set expectations.  */
>            if (p1[align1 + pos] < p2[align2 + pos])
>          result = -1;
>            else
>          result = 1;
>          }
>      }
> +
> +      /* Null terminate strings.  */
>        p1[len1 + align1] = 0;
>        p2[len2 + align2] = 0;
>
  

Patch

diff --git a/string/test-strncmp.c b/string/test-strncmp.c
index fe3c4e3..dabf1f1 100644
--- a/string/test-strncmp.c
+++ b/string/test-strncmp.c
@@ -270,33 +270,58 @@  do_random_tests (void)
    size_t i, j, n, align1, align2, pos, len1, len2, size;
    int result;
    long r;
+  /* Get pointers near the end of a "page".  */
    UCHAR *p1 = (UCHAR *) (buf1 + page_size - 512 * CHARBYTES);
    UCHAR *p2 = (UCHAR *) (buf2 + page_size - 512 * CHARBYTES);
  
    for (n = 0; n < ITERATIONS; n++)
      {
+      /* Start index for first string within p1.  */
        align1 = random () & 31;
+
+      /* Start index for second string within p2.  */
        if (random () & 1)
  	align2 = random () & 31;
        else
  	align2 = align1 + (random () & 24);
-      pos = random () & 511;
-      size = random () & 511;
+
+      /* Compute larger offset into buffers.  */
        j = align1 > align2 ? align1 : align2;
-      if (pos + j >= 511)
+
+      /* Position of difference between strings.  */
+      pos = random () & 511;
+
+      /* Ensure pos within range for strings.  */
+      if (j + pos >= 511)
  	pos = 510 - j - (random () & 7);
+
+      /* Maximum size of operation.  */
+      size = random () & 511;
+
+      /* Note: size does not need to be capped to the page boundary, as either
+	 - size >= pos, so the null ('\0') at 'pos' will terminate strncmp, or
+	 - size < pos, so 'size' will terminate strncmp.  */
+
+      /* Actual length of 1st string.  */
        len1 = random () & 511;
        if (pos >= len1 && (random () & 1))
  	len1 = pos + (random () & 7);
        if (len1 + j >= 512)
  	len1 = 511 - j - (random () & 7);
+
+      /* Actual length of 2nd string.  */
+      /* Note: len2 >= len1.  */
        if (pos >= len1)
  	len2 = len1;
        else
  	len2 = len1 + (len1 != 511 - j ? random () % (511 - j - len1) : 0);
+
+      /* Compute max length of strings, plus margin of dword.  */
        j = (pos > len2 ? pos : len2) + align1 + 64;
        if (j > 512)
  	j = 512;
+
+      /* Fill p1 with random data.  */
        for (i = 0; i < j; ++i)
  	{
  	  p1[i] = random () & 255;
@@ -307,6 +332,8 @@  do_random_tests (void)
  		p1[i] = 1 + (random () & 127);
  	    }
  	}
+
+      /* Fill p2 with random data.  */
        for (i = 0; i < j; ++i)
  	{
  	  p2[i] = random () & 255;
@@ -319,11 +346,15 @@  do_random_tests (void)
  	}
  
        result = 0;
+
+      /* Make strings the same, up to position pos.  */
        MEMCPY (p2 + align2, p1 + align1, pos);
+
        if (pos < len1)
  	{
  	  if (p2[align2 + pos] == p1[align1 + pos])
  	    {
+	      /* Insert difference.  */
  	      p2[align2 + pos] = random () & 255;
  	      if (p2[align2 + pos] == p1[align1 + pos])
  		p2[align2 + pos] = p1[align1 + pos] + 3 + (random () & 127);
@@ -331,12 +362,15 @@  do_random_tests (void)
  
  	  if (pos < size)
  	    {
+	      /* Set expectations.  */
  	      if (p1[align1 + pos] < p2[align2 + pos])
  		result = -1;
  	      else
  		result = 1;
  	    }
  	}
+
+      /* Null terminate strings.  */
        p1[len1 + align1] = 0;
        p2[len2 + align2] = 0;