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.


On 03/09/2017 01:28 AM, DJ Delorie wrote:
> 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.

Thanks for the information!

>
>> +  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.

I changed to set random chars at last minute before sending the patch. Indeed, it can be non-random to speed up the test.

>
>> +  /* 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.

Good catch. I meant to increase offset so that several alignments are tested.

I will send a v2 patch with these changes.

>
> 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)
>
Don't know too.


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