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

[Dmitry Samersoff]

Hi Everyone,

Updated webrev:

http://cr.openjdk.java.net/~dsamersoff/7017193/webrev.04/

Returned sscanf and addressed other comments.

-Dmitry



On 2011-02-25 01:43, Daniel D. Daugherty wrote:
> On 2/24/2011 2:24 PM, Dmitry Samersoff wrote:
>> [got mailer error for openjdk alias, so resending]
>>
>> Hi Everyone,
>>
>> Here is updated fix:
>>
>> http://cr.openjdk.java.net/~dsamersoff/7017193/webrev.03/
>
> src/os/linux/vm/os_linux.cpp
> line 2662: should be /proc/self/maps
>
> line 2663: literal '128' should be 'sizeof(buf)'
>
> line 2664: original logic looks for line ending with "[stack]"
> and your new logic matches a line with "[stack]" anywhere.
>
> line 2666: You assume that you got a number here. The original
> code would have returned false.
>
> line 2667: You assert if '-' is not after the first unsigned
> number translated. The orignal code would have returned
> false in case of such a mis-format.
>
> line 2668: You assume that 'np' is valid here (in product).
> The original code would have returned false.
> The second param to the second strtoul() call should be
> NULL or (char*) NULL.
>
>
> src/share/vm/runtime/os.cpp
> lines 1296-7 - space after '//'
>
> "const" on bsize parameter?
>
> line 1300: please consider adding this comment:
> // read until EOF, EOL or buf is full
>
> line 1305: please consider the following comment between lines
> 1306 and 1307 instead:
> // EOL reached so ignore EOL character and return
>
> lines 1313-4: please consider the following comment between lines
> 1315 and 1316 instead:
> // EOF reached. If we read chars before EOF, return them and
> // return EOF on next call. Otherwise, return EOF.
>
> line 1321: please move trailing ";" to next line, e.g.:
>
> while( read(fd, &ch, 1) == 1 && ch != '\n' )
> /* do nothing */ ;
>
> line 1323: please consider this comment instead:
>
> // Return initial part of line that fits in buf.
> // If we reached EOF, it will be returned on next call.
>
>
> src/share/vm/runtime/os.hpp
> "const" on bsize parameter?
>
> Why add blank lines on 725-6?
>
>
>>
>> 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 ...