Re: RFR: JDK-8161230 ClassLoader: add resource methods returning java.util.stream.Stream

[Tagir Valeev]

Hello!

The documentation says:

+ *  The search order is described in the documentation for
{@link+ * #getResource(String)}.


I think it means that the order of the stream is well-defined. In this case
should not we add ORDERED spliterator characteristic?

With best regards,
Tagir Valeev.

On Fri, Sep 2, 2016 at 3:06 AM, Patrick Reinhart  wrote:

> Hi Alan,
> Hi Paul,
>
> Here is the first revision of the implementation based on our earlier
> conversation.
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8161230/webrev.00 <
> http://cr.openjdk.java.net/~reinhapa/reviews/8161230/webrev.00>
>
> - Patrick
>

Re: RFR(m): 8159404: immutable collections should throw UOE unconditionally

[Paul Sandoz]

> On 1 Sep 2016, at 20:41, Martin Buchholz  wrote:
> 
> Looks good to me!
> 

+1

Paul.

Re: RFR(m): 8159404: immutable collections should throw UOE unconditionally

[Martin Buchholz]

Looks good to me!

Another idea for another day: I would like the immutable collections to be
more optimal than they currently are, even if we have to write more code.
It bugs me is that all of these collections have a modCount, despite never
being modified!  (Even for mutable lists, I'm not sure the added safety of
modCount was worth the price)

java -jar jol-cli-0.5-full.jar internals
java.util.ImmutableCollections\$List1
...
java.util.ImmutableCollections$List1 object internals:
  012(object header)N/A
 12 4int AbstractList.modCount  N/A
 16 4 Object List1.e0   N/A




On Thu, Sep 1, 2016 at 6:47 PM, Stuart Marks 
wrote:

> Hi all,
>
> Please review this change to make the immutable collections (List.of,
> Set.of, Map.of) throw UnsupportedOperationException unconditionally. That
> is, they should throw UOE even if a mutator method is called with arguments
> that make it a no-op. Calling a mutator method on an immutable collection
> is always a programming error, and having it sometimes be a no-op
> potentially leads to errors.
>
> Note that the existing Collections unmodifiable wrappers always throw UOE
> unconditionally. This change makes the immutable collections behave
> consistently with the unmodifiable wrappers. For example,
>
> List unmodList = Collections.unmodifiableList(new
> ArrayList<>());
> unmodList.addAll(List.of()); // throws UOE
> List.of().addAll(List.of()); // currently does nothing, change to
> throw UOE
>
> Unfortunately, various other specialized collections such as emptyList()
> etc. behave differently, e.g.
>
> Collections.emptyList().addAll(List.of()); // does nothing
>
> However, that change will be left for another day.
>
> Bug:
>
> https://bugs.openjdk.java.net/browse/JDK-8159404
>
> Webrev:
>
> http://cr.openjdk.java.net/~smarks/reviews/8159404/webrev.0/
>
> Thanks,
>
> s'marks
>

Re: RFR: 8081388: JNI exception pending in jdk/src/windows/bin/java_md.c

[David Holmes]

On 2/09/2016 2:03 AM, Henry Jen wrote:

That’s what I think too, this is just to silent parfait.


We are doing similar things in VM code, but the NULL check suffices. If 
that is not the case here then I think a Parfait bug needs to be filed.



I don’t know for sure that we always get NULL with exception pending though.


If you didn't that would be a VM bug. Either you get an array returned 
or you get NULL if the creation failed - the only way it can fail is if 
some exception is thrown.


Thanks,
David


Cheers,
Henry

On September 1, 2016 at 12:34:02 AM, David Holmes (david.hol...@oracle.com) 
wrote:

On 1/09/2016 5:51 AM, Henry Jen wrote:

Hi,

Please review a trivial fix for 8081388, in a nutshell,

- Return NULL from NewPlatformStringArray if an exception occurred
- All other places call this function already handled return value NULL
- Launcher handles exception in JavaMain, report error and exit.

Cheers,
Henry

diff --git a/src/java.base/share/native/libjli/java.c 
b/src/java.base/share/native/libjli/java.c
--- a/src/java.base/share/native/libjli/java.c
+++ b/src/java.base/share/native/libjli/java.c
@@ -1497,6 +1497,7 @@

NULL_CHECK0(cls = FindBootStrapClass(env, "java/lang/String"));
NULL_CHECK0(ary = (*env)->NewObjectArray(env, strc, cls, 0));
+ CHECK_EXCEPTION_RETURN_VALUE(0);


You will only get a NULL if an exception is pending; conversely you will
only have an exception pending if the return value is NULL. The new
check will never execute in a "positive way" and is unnecessary.

David
-


for (i = 0; i < strc; i++) {
jstring str = NewPlatformString(env, *strv++);
NULL_CHECK0(str);
diff --git a/src/java.base/share/native/libjli/java.h 
b/src/java.base/share/native/libjli/java.h
--- a/src/java.base/share/native/libjli/java.h
+++ b/src/java.base/share/native/libjli/java.h
@@ -253,6 +253,13 @@
#define NULL_CHECK(NC_check_pointer) \
NULL_CHECK_RETURN_VALUE(NC_check_pointer, )

+#define CHECK_EXCEPTION_RETURN_VALUE(CER_value) \
+ do { \
+ if ((*env)->ExceptionOccurred(env)) { \
+ return CER_value; \
+ } \
+ } while (JNI_FALSE)
+
#define CHECK_EXCEPTION_RETURN() \
do { \
if ((*env)->ExceptionOccurred(env)) { \

RFR(m): 8159404: immutable collections should throw UOE unconditionally

[Stuart Marks]

Hi all,

Please review this change to make the immutable collections (List.of, Set.of, 
Map.of) throw UnsupportedOperationException unconditionally. That is, they 
should throw UOE even if a mutator method is called with arguments that make it 
a no-op. Calling a mutator method on an immutable collection is always a 
programming error, and having it sometimes be a no-op potentially leads to errors.


Note that the existing Collections unmodifiable wrappers always throw UOE 
unconditionally. This change makes the immutable collections behave consistently 
with the unmodifiable wrappers. For example,


List unmodList = Collections.unmodifiableList(new ArrayList<>());
unmodList.addAll(List.of()); // throws UOE
List.of().addAll(List.of()); // currently does nothing, change to throw UOE

Unfortunately, various other specialized collections such as emptyList() etc. 
behave differently, e.g.


Collections.emptyList().addAll(List.of()); // does nothing

However, that change will be left for another day.

Bug:

https://bugs.openjdk.java.net/browse/JDK-8159404

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8159404/webrev.0/

Thanks,

s'marks

RE: RFR 9: JEP 290: Filter Incoming Serialization Data

[Peter Firmstone]

 
Hi Roger,
Many collections classes don't read arrays in directly, instead they read in 
the size as a primitive integer and create the array before reading in each 
object.
Clearly the filter can only prevent deserialization of dangerous objects.
My personal opinion is collections classes should be replaced in streams with 
safe to deserialize serial forms, that are just containers backed by arrays in 
object form  Client classes need to type check and defensively copy their 
content into new Collection object instances during deserialization. 
Filters might be able to validate these types, however in general, since there 
is no way to atomically fail construction during deserialization, object 
construction should be delayed as long as possible, that is, after all fields 
have deserialized unless a circular link exists.
Circular links should probably be avoided or their recursion depth limited in 
untrusted streams.
Regards,
Peter.
Original message:
Hi Peter,

Since the filter is passed information about each object created, a 
stateful filter can tabulate
the cumulative size itself if that is a concern.

Also, a stateless filter can be constructed to check a combination of 
the total number of objects,
depth, array sizes, and stream size. Since arrays are initialized with 
data from the stream,
the stream size provides a practical limit.

Roger

On 8/29/16 10:07 PM, Peter Firmstone wrote:
>Include original message
>
> A quick thought on the array size filter:
>
> The system creates an array with a size read from the stream.
>
> If Mallory sends a multidimensional array in the stream, then Mallory can 
> consume all jvm memory without exceeding the array size limit or the stream 
> data limit.
>
> We also need an array combined length limit.
>
> Thanks,
>
> Peter.
>
> Sent from my Samsung device.
>

Re: RFR 8164705: Remove pathname canonicalization from FilePermission

[Brian Burkhalter]

At the API level and conceptually this all appears reasonable. I am going to 
need to take a deeper pass over it all however to comprehend the implementation 
at any kind of detailed level. The changes mentioned in response to Alan’s 
comments all appear good.

Thanks,

Brian

On Sep 1, 2016, at 7:25 AM, Weijun Wang  wrote:

> Webrev updated at
> 
>   http://cr.openjdk.java.net/~weijun/8164705/webrev.01
> 
> Most suggestions from Alan accepted, including:
> 
> 1. Using SharedSecrets.getJavaIOFilePermissionAccess() style instead of 
> public functions.
> 
> 2. jdk.io.permissionsUseCanonicalPath=.
> 
> 3. samepath.sh rewritten in ReadFileOnPath.java.
> 
> Still using "ugly" method names. Thinking they are clear enough.

Re: RFR (JAXP) 8161454: Fails to Load external Java method from inside of a XSL stylesheet if SecurityManager is present

[Joe Wang]

Thanks Daniel!

-Joe

On 9/1/16, 3:37 PM, Daniel Fuchs wrote:

On 31/08/16 21:47, Joe Wang wrote:

Thanks Aleksej!


Hi Joe,

Your last revision looks good.

best,

-- daniel


-Joe

On 8/31/16, 11:26 AM, Aleks Efimov wrote:

Hi Joe,

The changes looks nice (I'm not a reviewer)


Best Regards,

Aleksej


On 31/08/16 19:47, Joe Wang wrote:

Hi,

Please review a fix to the XSLTFunctionsTest. After enabling
SecurityManager, the test now needs to set the extension ClassLoader
for the extension class.

JBS: https://bugs.openjdk.java.net/browse/JDK-8161454
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8161454/webrev/

Thanks,
Joe

Re: RFR (JAXP) 8161454: Fails to Load external Java method from inside of a XSL stylesheet if SecurityManager is present

[Daniel Fuchs]

On 31/08/16 21:47, Joe Wang wrote:

Thanks Aleksej!


Hi Joe,

Your last revision looks good.

best,

-- daniel


-Joe

On 8/31/16, 11:26 AM, Aleks Efimov wrote:

Hi Joe,

The changes looks nice (I'm not a reviewer)


Best Regards,

Aleksej


On 31/08/16 19:47, Joe Wang wrote:

Hi,

Please review a fix to the XSLTFunctionsTest. After enabling
SecurityManager, the test now needs to set the extension ClassLoader
for the extension class.

JBS: https://bugs.openjdk.java.net/browse/JDK-8161454
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8161454/webrev/

Thanks,
Joe

RFR: JDK-8161230 ClassLoader: add resource methods returning java.util.stream.Stream

[Patrick Reinhart]

Hi Alan,
Hi Paul,

Here is the first revision of the implementation based on our earlier 
conversation.

http://cr.openjdk.java.net/~reinhapa/reviews/8161230/webrev.00 


- Patrick

RFR: JDK-8165161 Solaris: /usr/ccs /opt/sfw and /opt/csw are dead, references should be expunged

[Alan Burlison]

I posted this originally on build-dev, it was suggested I should also 
post it to core-libs-dev for review of some of the changes.


/usr/ccs /opt/sfw and /opt/csw are all obsolete and should be removed 
from the Solaris-related build infrastructure.


Bug:https://bugs.openjdk.java.net/browse/JDK-8165161
Webrev: http://cr.openjdk.java.net/~alanbur/JDK-8165161/

JPRT hotspot tests all pass.

Thanks,

--
Alan Burlison
--

Re: [jdk9] (XS) RFR: 8165243: Base64.Encoder.wrap(os).write(byte[], int, int) with incorrect arguments should not produce output

[Alan Bateman]

On 01/09/2016 17:40, Ivan Gerasimov wrote:


Hello!

An encoding output stream 'es' can be obtained as encoder.wrap(os).
Normally, es.write(buf, off, len) throws IndexOutOfBoundException, if 
the referenced portion lies outside of the buffer.

In this case, nothing is written to the underlying output stream 'os'.

However, if (off + len) overflows, then the call to write() will 
produce some output, and only when it reaches the boundary of the buf 
will it throw the exception.


This behavior looks inconsistent.

Would you please help review the simple fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8165243
WEBREV: http://cr.openjdk.java.net/~igerasim/8165243/00/webrev/
TestBase64 is the unit test for this API, should the test be added to 
that instead?


-Alan

Re: [jdk9] (XS) RFR: 8165243: Base64.Encoder.wrap(os).write(byte[], int, int) with incorrect arguments should not produce output

[Roger Riggs]

Looks fine Ivan.

Thanks, Roger


On 9/1/2016 12:40 PM, Ivan Gerasimov wrote:

Hello!

An encoding output stream 'es' can be obtained as encoder.wrap(os).
Normally, es.write(buf, off, len) throws IndexOutOfBoundException, if 
the referenced portion lies outside of the buffer.

In this case, nothing is written to the underlying output stream 'os'.

However, if (off + len) overflows, then the call to write() will 
produce some output, and only when it reaches the boundary of the buf 
will it throw the exception.


This behavior looks inconsistent.

Would you please help review the simple fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8165243
WEBREV: http://cr.openjdk.java.net/~igerasim/8165243/00/webrev/

With kind regards,
Ivan

[jdk9] (XS) RFR: 8165243: Base64.Encoder.wrap(os).write(byte[], int, int) with incorrect arguments should not produce output

[Ivan Gerasimov]

Hello!

An encoding output stream 'es' can be obtained as encoder.wrap(os).
Normally, es.write(buf, off, len) throws IndexOutOfBoundException, if 
the referenced portion lies outside of the buffer.

In this case, nothing is written to the underlying output stream 'os'.

However, if (off + len) overflows, then the call to write() will produce 
some output, and only when it reaches the boundary of the buf will it 
throw the exception.


This behavior looks inconsistent.

Would you please help review the simple fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8165243
WEBREV: http://cr.openjdk.java.net/~igerasim/8165243/00/webrev/

With kind regards,
Ivan

Re: RFR: 8081388: JNI exception pending in jdk/src/windows/bin/java_md.c

[Henry Jen]

That’s what I think too, this is just to silent parfait.
I don’t know for sure that we always get NULL with exception pending though.

Cheers,
Henry

On September 1, 2016 at 12:34:02 AM, David Holmes (david.hol...@oracle.com) 
wrote:
> On 1/09/2016 5:51 AM, Henry Jen wrote:
> > Hi,
> >
> > Please review a trivial fix for 8081388, in a nutshell,
> >
> > - Return NULL from NewPlatformStringArray if an exception occurred
> > - All other places call this function already handled return value NULL
> > - Launcher handles exception in JavaMain, report error and exit.
> >
> > Cheers,
> > Henry
> >
> > diff --git a/src/java.base/share/native/libjli/java.c 
> > b/src/java.base/share/native/libjli/java.c  
> > --- a/src/java.base/share/native/libjli/java.c
> > +++ b/src/java.base/share/native/libjli/java.c
> > @@ -1497,6 +1497,7 @@
> >
> > NULL_CHECK0(cls = FindBootStrapClass(env, "java/lang/String"));
> > NULL_CHECK0(ary = (*env)->NewObjectArray(env, strc, cls, 0));
> > + CHECK_EXCEPTION_RETURN_VALUE(0);
>  
> You will only get a NULL if an exception is pending; conversely you will
> only have an exception pending if the return value is NULL. The new
> check will never execute in a "positive way" and is unnecessary.
>  
> David
> -
>  
> > for (i = 0; i < strc; i++) {
> > jstring str = NewPlatformString(env, *strv++);
> > NULL_CHECK0(str);
> > diff --git a/src/java.base/share/native/libjli/java.h 
> > b/src/java.base/share/native/libjli/java.h  
> > --- a/src/java.base/share/native/libjli/java.h
> > +++ b/src/java.base/share/native/libjli/java.h
> > @@ -253,6 +253,13 @@
> > #define NULL_CHECK(NC_check_pointer) \
> > NULL_CHECK_RETURN_VALUE(NC_check_pointer, )
> >
> > +#define CHECK_EXCEPTION_RETURN_VALUE(CER_value) \
> > + do { \
> > + if ((*env)->ExceptionOccurred(env)) { \
> > + return CER_value; \
> > + } \
> > + } while (JNI_FALSE)
> > +
> > #define CHECK_EXCEPTION_RETURN() \
> > do { \
> > if ((*env)->ExceptionOccurred(env)) { \
> >
>  

Re: Last Ant Test Failure with JDK9 - JAXP Secure Processing and XSLT Extensions

[Stefan Bodewig]

On 2016-08-31, Joe Wang wrote:

> On 8/30/16, 9:34 AM, Stefan Bodewig wrote:

>> On 2016-08-29, Joe Wang wrote:

>>> If you are using the built-in extension functions, try turning on the
>>> following feature:
>>>  private static final String ENABLE_EXTENSION_FUNCTIONS =
>>> "http://www.oracle.com/xml/jaxp/properties/enableExtensionFunctions;;
>>>  tf.setFeature(ENABLE_EXTENSION_FUNCTIONS, true);
>> This is not supported by Xalan's TransformerFactoryImpl:

> True, this is an impl-only feature. But Xalan doesn't need it anyways,
> you may check the factory instance and skip it if it's Xalan.

Thanks, you're comment made me take a third look at the test-case in
question. I was confused by the setup and overlooked that we explicitly
forced the use of the JDK's factory for just a single test.

By selectively setting both features I can get the test to pass and am
able to use the redirect extension of a version of Xalan on the
classloader I specify.

I'll need to add suppport for setting features on the TransformerFactory
to Ant's  task as I'd prefer to not hard-code the features into
the task - and enable it for  be default.

>> When removing Xalan from the classpath and using the JDK's own
>> TransformerFactory I get

>> ,
>> | Error! Use of the extension element 'redirect' is not allowed when the
>> | secure processing feature is set to true.
>> `

>> even with the feature enabled. So "redirect" -
>> i.e. xmlns:redirect="http://xml.apache.org/xalan/redirect; - which I
>> assumed to be "built-in" for the JDK's fork of Xalan as well - doesn't
>> seem to get through with just that.

> I'll get this fixed in the next 1 or 2
> build. (https://bugs.openjdk.java.net/browse/JDK-8165116)

This is great and will simplify things a lot.

Many thanks

 Stefan

Re: RFR 8164705: Remove pathname canonicalization from FilePermission

[Weijun Wang]

Webrev updated at

   http://cr.openjdk.java.net/~weijun/8164705/webrev.01

Most suggestions from Alan accepted, including:

1. Using SharedSecrets.getJavaIOFilePermissionAccess() style instead of 
public functions.


2. jdk.io.permissionsUseCanonicalPath=.

3. samepath.sh rewritten in ReadFileOnPath.java.

Still using "ugly" method names. Thinking they are clear enough.

Thanks
Max

On 8/29/2016 20:18, Alan Bateman wrote:

On 25/08/2016 04:55, Weijun Wang wrote:


Hi All

Please take a look at

   http://cr.openjdk.java.net/~weijun/8164705/webrev.00

From the beginning of JDK, FilePermission canonicalizes the input path
and use the result in implies() and equals(). This has been a big
performance hurt and leads to confusing results when symlinks are
involved.

The code change above removes the canonicalization.

This means FilePermission on "/the/current/working/directory/x" no
longer implies that on "x". Since this might bring quite some
compatibility risk, the code change includes some tweaks in permission
checking to make sure an app is still able to read "x" when the
FilePermission granted is on "/the/current/working/directory/x".
However, we still hope the policy to be updated to be consistent of
how a file is actually accessed.

No tweak is devoted to make granting "/this/is/a/symlink" to imply
reading of "/the/actual/target/file", because we think it should not.

This is quite a big behavior change. If it breaks your app/lib, or
does not work with your customized security manager or policy
implementation, please let us know.

Feel free to provide any feedback.

Finally, a new system property "jdk.filepermission.canonicalize" is
introduced and it can be "true", "false", or "compat". The out-of-box
default is "compat", which means no canonicalization with check tweaks.

I've taken a first pass over this. A general comment then it's great to
see FilePermission changed to not do canonicalization. The "new
behavior" approach to match paths and support absolute => relative and
relative => absolute all makes sense.

I agree that having a property to revert to legacy behavior make sense.
Is there any way to reduce this down to just current and legacy
behavior, it's otherwise too complicated to describe the subtle
differences between "false" and "compat". If it can be reduced to two
settings then a name such as
jdk.io.permissionsUseCanonicalPath= could be better than the
prototype name.

For the implementation then I think it will cleanup before this is ready
to push. FilePermCompat is a ugly, particularly FilePermission changing
its public fields. Also method names like impliesWithAltFP,
newPermPlusAltPath, ... could be re-visited to make them much clearer.

For the test then it would be good if samepath.sh could be replaced with
a non-shell test.

-Alan

Re: RFR: JDK-8161360,,Deprecated vfork() should not be used on Solaris

[Alan Burlison]

On 01/09/2016 14:31, Dmitry Samersoff wrote:


Changes looks good for me.


Thanks.

Could someone possibly sponsor this for me? I don't have commit rights 
yet...


--
Alan Burlison
--

Re: RFR: JDK-8161360,,Deprecated vfork() should not be used on Solaris

[Dmitry Samersoff]

Alan,

Changes looks good for me.

-Dmitry

On 2016-08-31 14:55, Alan Burlison wrote:
> vfork(2) is deprecated on Solaris and using it generates compiler
> warnings. When compiled with warnings-as-errors, this results in
> compilation failures.
> 
> Bug:https://bugs.openjdk.java.net/browse/JDK-8161360
> Webrev: http://cr.openjdk.java.net/~alanbur/JDK-8161360
> 
> Thanks,
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.

Re: RFR: JDK-8161360, , Deprecated vfork() should not be used on Solaris

[Dmitry Samersoff]

Martin,

Valid launch mechanisms for Solaris set in ProcessImpl.java as:

  SOLARIS(LaunchMechanism.POSIX_SPAWN, LaunchMechanism.FORK),

So it's not possible to set VFORK as LaunchMechanism for Solaris.
and this fix doesn't change anything from customer perspective.

-Dmitry


On 2016-09-01 04:55, Martin Buchholz wrote:
> Does an attempt to use vfork on Solaris result in something reasonable like
> UnsupportedOperationException?
> 
> On Wed, Aug 31, 2016 at 4:55 AM, Alan Burlison 
> wrote:
> 
>> vfork(2) is deprecated on Solaris and using it generates compiler
>> warnings. When compiled with warnings-as-errors, this results in
>> compilation failures.
>>
>> Bug:https://bugs.openjdk.java.net/browse/JDK-8161360
>> Webrev: http://cr.openjdk.java.net/~alanbur/JDK-8161360
>>
>> Thanks,
>>
>> --
>> Alan Burlison
>> --
>>


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.

Re: RFR: JDK-8161360,,Deprecated vfork() should not be used on Solaris

[Alan Burlison]

On 01/09/2016 02:55, Martin Buchholz wrote:


Does an attempt to use vfork on Solaris result in something reasonable like
UnsupportedOperationException?


Yes:

$ jshell
|  Welcome to JShell -- Version 9-internal
|  For an introduction type: /help intro


jshell> System.setProperty("jdk.lang.Process.launchMechanism", "VFORK")
$1 ==> null

jshell> Runtime.getRuntime().exec("ls");
|  java.lang.Error thrown: VFORK is not a supported process launch 
mechanism on this platform.
|at ProcessImpl$Platform.lambda$launchMechanism$0 
(ProcessImpl.java:151)

|at AccessController.doPrivileged (Native Method)
|at ProcessImpl$Platform.launchMechanism (ProcessImpl.java:134)
|at ProcessImpl. (ProcessImpl.java:174)
|at ProcessBuilder.start (ProcessBuilder.java:1107)
|at ProcessBuilder.start (ProcessBuilder.java:1071)
|at Runtime.exec (Runtime.java:635)
|at Runtime.exec (Runtime.java:459)
|at Runtime.exec (Runtime.java:356)
|at (#2:1)

jshell>

--
Alan Burlison
--

Re: RFR: 8081388: JNI exception pending in jdk/src/windows/bin/java_md.c

[David Holmes]

On 1/09/2016 5:51 AM, Henry Jen wrote:

Hi,

Please review a trivial fix for 8081388, in a nutshell,

- Return NULL from NewPlatformStringArray if an exception occurred
- All other places call this function already handled return value NULL
- Launcher handles exception in JavaMain, report error and exit.

Cheers,
Henry

diff --git a/src/java.base/share/native/libjli/java.c 
b/src/java.base/share/native/libjli/java.c
--- a/src/java.base/share/native/libjli/java.c
+++ b/src/java.base/share/native/libjli/java.c
@@ -1497,6 +1497,7 @@

 NULL_CHECK0(cls = FindBootStrapClass(env, "java/lang/String"));
 NULL_CHECK0(ary = (*env)->NewObjectArray(env, strc, cls, 0));
+CHECK_EXCEPTION_RETURN_VALUE(0);


You will only get a NULL if an exception is pending; conversely you will 
only have an exception pending if the return value is NULL. The new 
check will never execute in a "positive way" and is unnecessary.


David
-


 for (i = 0; i < strc; i++) {
 jstring str = NewPlatformString(env, *strv++);
 NULL_CHECK0(str);
diff --git a/src/java.base/share/native/libjli/java.h 
b/src/java.base/share/native/libjli/java.h
--- a/src/java.base/share/native/libjli/java.h
+++ b/src/java.base/share/native/libjli/java.h
@@ -253,6 +253,13 @@
 #define NULL_CHECK(NC_check_pointer) \
 NULL_CHECK_RETURN_VALUE(NC_check_pointer, )

+#define CHECK_EXCEPTION_RETURN_VALUE(CER_value) \
+do { \
+if ((*env)->ExceptionOccurred(env)) { \
+return CER_value; \
+} \
+} while (JNI_FALSE)
+
 #define CHECK_EXCEPTION_RETURN() \
 do { \
 if ((*env)->ExceptionOccurred(env)) { \