This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add page tests to string/test-strnlen.


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)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]