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

[Artem Ananiev]

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
>