Request for review (S, UPDATED) 7017193: Small memory leak in get_stack_bounds // os::create_stack_guard_pages

[Dmitry Samersoff]

David,

I'm strictly against reading /proc entry using stdio functions,
as (a) /proc file could be changed while we are reading it (b) it's not 
fancy as we are buffering kmem.

/proc/self/maps has a fixed format and if we have a bad line in a /proc 
file it would mean that something fatal happens with system, but I'll 
change assert to "return false" so the code will silently bail out in 
case of malformed line.

-Dmitry


On 2011-02-25 01:07, David Holmes wrote:
> 2662 // Address part of /proc/maps couldn't be more than 128 bytes
> 2663 while ( os::get_line_chars(fd, buf, 128) > 0) {
> 2664 if (::strstr(buf, "[stack]") != 0) {
> 2665 // Extract addresses
> 2666 *bottom = ::strtoul(buf, &np, 16);
> 2667 assert(*np == '-', "Should be a dash");
> 2668 *top = ::strtoul(np + 1, 0, 16);
>
> Don't you need to verify that you actually read data into the parts of
> the buf you're attempting to convert. The line might be malformed for
> some reason, or truncated. And why not just keep the sscanf code?



>
> David
>
> Dmitry Samersoff said the following on 02/25/11 07:24:
>> [got mailer error for openjdk alias, so resending]
>>
>> Hi Everyone,
>>
>> Here is updated fix:
>>
>> http://cr.openjdk.java.net/~dsamersoff/7017193/webrev.03/
>>
>> I introduce a new convenience function primary designed to read
>> variety of /proc files. It reads first N characters of line and then
>> skip until eol.
>>
>> Than get_stack_bounds() is changed to use it instead of stdio getline();
>>
>> -Dmitry
>>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...