<AWT Dev> [9] Review request : 6788138 leak in Java_sun_awt_X11_XlibWrapper_getStringBytes?

[Petr Pchelko]

Hello, Alexander.

Looks good.

With best regards. Petr.

On 04.04.2014, at 18:40, Alexander Zvegintsev <alexander.zvegintsev at oracle.com> wrote:

> Hello AWT team,
> 
> Could I get a second review?
> 
> Thanks,
> 
> Alexander.
> 
> On 03/25/2014 08:07 PM, Sergey Bylokhov wrote:
>> On 3/21/14 3:10 PM, Alexander Zvegintsev wrote:
>>> Hi Sergey,
>>> 
>>> Updated:
>>> http://cr.openjdk.java.net/~azvegint/jdk/9/6788138/01/
>>> 
>>>> - This two line was added by the dm, in the fix for "Fixed size problem in getStringBytes" in 2003. Note that the storage has "length +1" size. Why? 
>>> 
>>> May be there was an attempt to use a NULL-terminated string(why?), but since we passing to SetByteArrayRegion length( not length + 1) it makes no sense.
>>> We passing result of getStringBytes to a String constructor, so we don't need a trailing \0.
>> Thanks. The fix looks fine to me.
>>> 
>>> Thanks,
>>> 
>>> Alexander.
>>> 
>>> On 03/21/2014 02:51 PM, Sergey Bylokhov wrote:
>>>> Hi, Alexander.
>>>> A few notes.
>>>> - documentation of this method is wrong.(copypaste from XFree)
>>>> - This two line was added by the dm, in the fix for "Fixed size problem in getStringBytes" in 2003. Note that the storage has "length +1" size. Why?
>>>> 
>>>> On 3/21/14 2:27 PM, Alexander Zvegintsev wrote:
>>>>> Hello AWT team,
>>>>> 
>>>>> please review the fix
>>>>> http://cr.openjdk.java.net/~azvegint/jdk/9/6788138/00/
>>>>> for the issue
>>>>> https://bugs.openjdk.java.net/browse/JDK-6788138
>>>>> 
>>>>> 
>>>>> Currently we use this function to get Window Manager name from native only.
>>>>> It happens only once at startup, hence this leak is not noticeable.
>>>>> 
>>>> 
>>>> 
>>> 
>> 
>> 
>