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

[Daniel D. Daugherty]

On 2/25/2011 11:57 AM, Dmitry Samersoff wrote:
> Hi Everyone,
>
> Updated webrev:
>
> http://cr.openjdk.java.net/~dsamersoff/7017193/webrev.04/

src/os/linux/vm/os_linux.cpp
    nit on line 2656: add space after ','
    nit on line 2666: HotSpot style is 'while (('

src/share/vm/runtime/os.cpp
    nit on line 1299: add space after ','
    nit on line 1302: HotSpot style is 'while (('
    nit on line 1302: add spaces around '=' => ' = '
    nit on line 1324: HotSpot style is 'while ('

src/share/vm/runtime/os.hpp
    No comments.


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