<AWT Dev> [8] Review request for 8025775: JNI warnings in TryXShmAttach

[Anthony Petrov]

+1

--
best regards,
Anthony

On 12/02/2013 06:54 PM, Artem Ananiev wrote:
> Hi, Anton,
>
> the new version is fine. Thanks for addressing my comments,
>
> Artem
>
> On 12/2/2013 6:52 PM, Anton Litvinov wrote:
>> Hello Artem,
>>
>> Thank you for review of this fix. Could you please review the second
>> version of the fix, which should follow your remarks.
>>
>> Webrev: http://cr.openjdk.java.net/~alitvinov/8025775/jdk8/webrev.01
>>
>> Differences between this version of the fix and the previous version are
>> following:
>>
>> 1.  The unused variable "xerror_code" was removed from "awt_util.h",
>> "awt_util.c" files.
>> 2.  The unused macro "XERROR_SAVE" was removed from "awt_util.h" file.
>> 3.  Declaration of the external variable "current_native_xerror_handler"
>> was removed from "XlibWrapper.c" file, however the next line was added
>> to "XlibWrapper.c" file to be able to compile JDK.
>>
>>      44 #include <awt_util.h>
>>
>> 4.  In the file "XlibWrapper.c" the comment was changed from
>>
>>      1270     // Pass the event to a native synthetic error handler
>> first.
>> to
>>      1270     // First call the native synthetic error handler declared
>> in "awt_util.h" file.
>>
>> Thank you,
>> Anton
>>
>> On 12/2/2013 12:19 PM, Artem Ananiev wrote:
>>> Hi, Anton,
>>>
>>> the fix looks fine in general. A few minor comments:
>>>
>>> 1. xerror_code is not used now and can be removed. XERROR_SAVE seems
>>> to be unused as well.
>>>
>>> 2. current_native_xerror_handler can be declared as "extern" in
>>> awt_util.h, so you don't need to declare it in every file where it's
>>> used (e.g. in XlibWrapper.c)
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>> On 11/30/2013 2:13 AM, Anton Litvinov wrote:
>>>> Hello,
>>>>
>>>> Could you please review the following fix for the bug, which
>>>> consists in
>>>> invocation of Java methods from native code via JNI ("TryXShmAttach"
>>>> function in "awt_GraphicsEnv.c" file) after "GetPrimitiveArrayCritical"
>>>> is called ("BufImg_GetRasInfo" function in "BufImgSurfaceData.c" file).
>>>>
>>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8025775
>>>> Webrev: http://cr.openjdk.java.net/~alitvinov/8025775/jdk8/webrev.00
>>>>
>>>> The solution is based on:
>>>> - Reversion of all XError handling code implemented as "native -> Java"
>>>> calls in the fix for CR 8005607 to its prior state with native error
>>>> handlers.
>>>> - Introduction of a native synthetic error handler
>>>> "awt_util.c::current_native_xerror_handler" set/unset through
>>>> previously
>>>> known macros in "awt_util.h" file and called from
>>>> "XlibWrapper.c::ToolkitErrorHandler" function before a call to Java
>>>> synthetic error handler.
>>>>
>>>> The fix also changed both returned native error handlers:
>>>> "awt_GraphicsEnv.c::XShmAttachXErrHandler",
>>>> "GLXSurfaceData.c::GLXSD_BadAllocXErrHandler" not to call previous
>>>> error
>>>> handler and to always return "0" value as a result.
>>>>
>>>> Thank you,
>>>> Anton
>>