Code review request: 7152671,RFE: Windows decoder should add some std dirs to the symbol search path

[Daniel D. Daugherty]

On 1/8/13 6:06 PM, Zhengyu Gu wrote:
> Hi Dan,
>
> Thank you for reviewing.

You're very welcome.


>
> On 1/8/2013 6:03 PM, Daniel D. Daugherty wrote:
>> On 1/8/13 1:34 PM, Zhengyu Gu wrote:
>>> This is an enhancement that allows Windows decoder to search pdb 
>>> files that are created by FDS.
>>>
>>> In short, if FDS pdb files are downloaded into following paths, 
>>> Widnows decoder should be able to use them to decode stacks:
>>>
>>> $JDK/lib
>>> $JRE/lib
>>> directory where jvm.dll is loaded from.  ($JRE/bin/{server or client})
>>>
>>>
>>> http://cr.openjdk.java.net/~zgu/7152671/webrev.01/
>>
>> src/os/windows/vm/decoder_windows.hpp
>>     No comments.
>>
>> src/os/windows/vm/decoder_windows.cpp
>>     line 26: why add a blank line?
>>
>>     lines 76-77: these seem a little longer than 80...
>>
> Will fix them.

Thanks.


>
>>     line 80: int  len = sizeof(paths);
>>         Should be sizeof(paths) - 1 to leave room for the NULL 
>> terminator.
>>         Otherwise, your final strncat() call in the sequence could 
>> append
>>         a NULL and overflow the paths buffer.
> I do realize the string will be terminated by extra '\0'. It is 
> handled by remaining buffer size checking code, which has "len > 
> number of characters to be appended", so it is guaranteed an extra 
> space for '\0'

Agreed. Your existing logic is correct.


>
>>
>>     line 130:  if (dwAttrib != INVALID_FILE_ATTRIBUTES &&
>>     line 131:      dwAttrib & FILE_ATTRIBUTE_DIRECTORY)
>>         Putting parens around the bit-mask on line 131 would
>>         be more clear; not required, but more clear.
> Will do.

Thanks.

Dan

>
> Thanks,
>
> -Zhengyu
>>
>> Dan
>>
>>
>>
>>>
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>>
>>