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

[Baesken, Matthias]

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.