This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add page tests to string/test-strnlen.
- From: DJ Delorie <dj at redhat dot com>
- To: Wainer dos Santos Moschetta <wainersm at linux dot vnet dot ibm dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 08 Mar 2017 23:28:41 -0500
- Subject: Re: [PATCH] Add page tests to string/test-strnlen.
- Authentication-results: sourceware.org; auth=none
Wainer dos Santos Moschetta <wainersm@linux.vnet.ibm.com> writes:
> + /* Calculate the null character offset. */
> + size_t last_offset = (page_size / sizeof (CHAR)) - 1;
> +
> + CHAR *s = (CHAR *) buf1;
Note that buf1 is BUF1PAGES pages long, not always one page.
s[last_offset] is not at the end of the mmap'd area if BUF1PAGES is
overridden.
buf2 is always one page long, though.
> + for (i = 0; i < last_offset; i++)
> + s[i] = (random() & 127) + 1;
Does this need to be random, or is filling it with 'x' sufficient? I'm
just thinking of speed here, and what the optimizer may do with known
non-random data.
> + /* Place long strings ending at page boundary. */
> + offset = (last_offset + 1) / 2;
> + for (i = 0; i < 64; ++i)
> + {
> + offset += i;
By the end of the loop, offset will be 2048 bytes beyond its starting
point - did you intend for offset to move exponentially? Or did you
mean "offset = starting_offset + i" ? Either way, a comment stating
your intentions would be suitable here.
The rest of it looks OK to me. However, I wonder if any of these
mmap-based tests have some way of forcing a page violation (by
intentionally accessing beyond the mmap'd area) to test to be sure that
an exception actually happens? If the test can't prove the fault will
happen, all the other tests are technically inconclusive. (not that
it's relevent to the patch you're submitting, I'm just wondering)