<AWT Dev> RFR: [XS] 8214380: AwtDragSource function LoadCache misses a ReleaseLongArrayElements in special case

[Sergey Bylokhov]

Looks fine.

On 30/11/2018 05:22, Baesken, Matthias wrote:
> Hi Sergey,   I prepared a second webrev  :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8214380.1/
> 
> - removed the unused isCopy
> - added a NULL check  after  GetLongArrayElements
> 
> 
> Best regards, Matthias
> 
> 
> 
>> -----Original Message-----
>> From: Baesken, Matthias
>> Sent: Donnerstag, 29. November 2018 10:01
>> To: 'Sergey Bylokhov' <Sergey.Bylokhov at oracle.com>; 'awt-
>> dev at openjdk.java.net' <awt-dev at openjdk.java.net>
>> Subject: RE: <AWT Dev> RFR: [XS] 8214380: AwtDragSource function
>> LoadCache misses a ReleaseLongArrayElements in special case
>>
>> Btw when looking at the coding,  I  wonder  why  we save the copy-state  in
>> isCopy  :
>>
>> Docu :    "If isCopy is not NULL, then *isCopy is set to JNI_TRUE if a copy is
>> made; or it is set to JNI_FALSE if no copy is made"
>>
>>   397     jboolean isCopy;
>>   398     jlong *lFormats = env->GetLongArrayElements(formats, &isCopy),
>>
>>   But  then we do not use it afterwards ,   so should we better call   it with
>> NULL  instead?
>>
>> Regards, Matthias
>>
>>
>>> -----Original Message-----
>>> From: Baesken, Matthias
>>> Sent: Donnerstag, 29. November 2018 09:17
>>> To: 'Sergey Bylokhov' <Sergey.Bylokhov at oracle.com>; awt-
>>> dev at openjdk.java.net
>>> Subject: RE: <AWT Dev> RFR: [XS] 8214380: AwtDragSource function
>>> LoadCache misses a ReleaseLongArrayElements in special case
>>>
>>> Hi Sergey,   I  think it is (at least)  a good practice  to NULL -  check  the result
>>> of  env->GetLongArrayElements .
>>> Probably it should be done  right after  the
>>>
>>>       jlong *lFormats = env->GetLongArrayElements(formats, &isCopy),
>>>
>>> call.
>>> See  jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c   for
>>> examples of usages   of the function (with NULL checks) .
>>>
>>> Or see the documentation at
>>>
>> https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/function
>>> s.html
>>> Where we find :
>>>
>>>
>>> Get<PrimitiveType>ArrayElements Routines
>>>     ....
>>> PARAMETERS:
>>>
>>> env: the JNI interface pointer.
>>> array: a Java string object.
>>> isCopy: a pointer to a boolean.
>>>
>>>
>>> RETURNS:
>>>
>>> Returns a pointer to the array elements, ****or NULL if the operation
>>> fails.****
>>>
>>>
>>> So returning NULL might  happen .
>>>
>>> Best regards, Matthias
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Sergey Bylokhov <Sergey.Bylokhov at oracle.com>
>>>> Sent: Donnerstag, 29. November 2018 00:22
>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; awt-
>>>> dev at openjdk.java.net
>>>> Subject: Re: <AWT Dev> RFR: [XS] 8214380: AwtDragSource function
>>>> LoadCache misses a ReleaseLongArrayElements in special case
>>>>
>>>> Hi, Matthias.
>>>>
>>>> Do we need the null check in the fix, if "yes" then probably
>>>> the same check should be added before this line as well?
>>>>     465     env->ReleaseLongArrayElements(formats, saveFormats, 0);
>>>>
>>>> On 28/11/2018 00:33, Baesken, Matthias wrote:
>>>>> Hello, please review  this small fix.
>>>>>
>>>>> It adds a missing ReleaseLongArrayElements  to
>>>>>
>>>>> void AwtDragSource::LoadCache(jlongArray formats)
>>>>>
>>>>> in an early special   "pseudo return"  (leave function via throw)  case.
>>>>>
>>>>> Webrev/bug :
>>>>>
>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8214380.0/
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8214380
>>>>>
>>>>> Thanks, Matthias
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.


-- 
Best regards, Sergey.