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.bylok...@oracle.com>; 'awt-
d...@openjdk.java.net' <awt-dev@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.bylok...@oracle.com>; awt-
d...@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.bylok...@oracle.com>
Sent: Donnerstag, 29. November 2018 00:22
To: Baesken, Matthias <matthias.baes...@sap.com>; awt-
d...@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.
