Re: RFR: 8219548 Re: Faulty Null-Check Suspected in ToolProvider

Looks good to me. -- Jon On 2/21/19 12:34 PM, Lance Andersen wrote: Here is the webrev of the patch from Philipp. I did not change the name of the test() method as it was not necessary http://cr.openjdk.java.net/~lancea/8219548/webrev.00/ On Feb 15, 2019, at 6:59 PM, Philipp Kunz wrote:

Re: RFR: 8219548 Re: Faulty Null-Check Suspected in ToolProvider

On 21/02/2019 20:34, Lance Andersen wrote: Here is the webrev of the patch from Philipp. I did not change the name of the test() method as it was not necessary http://cr.openjdk.java.net/~lancea/8219548/webrev.00/ This looks okay to me, probably should get Jon to review as he added this API.

RFR: 8219548 Re: Faulty Null-Check Suspected in ToolProvider

Here is the webrev of the patch from Philipp. I did not change the name of the test() method as it was not necessary http://cr.openjdk.java.net/~lancea/8219548/webrev.00/ > On Feb 15, 2019, at 6:59 PM, Philipp Kunz wrote: > > Hi Lance, > > See attached patch. > > Regards, > Philipp > > >

Re: Faulty Null-Check Suspected in ToolProvider

its on my todo list as is looking at your other issue, just have not gotten to it yet :-) > On Feb 20, 2019, at 1:44 PM, Philipp Kunz wrote: > > Hi Jon, > > All right, let's not document flushing behavior then. It's probably really > not important enough. So we're back to the null-checks? > F

Re: Faulty Null-Check Suspected in ToolProvider

Hi Jon, All right, let's not document flushing behavior then. It's probably really not important enough. So we're back to the null-checks?For that see patch of https://mail.openjdk.java.net/pipermail/core-libs-dev/2019 -February/058576.html Regards,Philipp On Sat, 2019-02-16 at 17:05 -0800, Jonath

Re: Faulty Null-Check Suspected in ToolProvider

I still don't see why it is necessary to specify this behavior. -- Jon On 2/16/19 4:45 PM, Philipp Kunz wrote: Hi Jon, On Sat, 2019-02-16 at 13:44 -0800, Jonathan Gibbons wrote: On 2/16/19 12:20 AM, Philipp Kunz wrote: I'm also wondering about the call to flush in run(PrintStream out, PrintS

Re: Faulty Null-Check Suspected in ToolProvider

Hi Jon, On Sat, 2019-02-16 at 13:44 -0800, Jonathan Gibbons wrote: > On 2/16/19 12:20 AM, Philipp Kunz wrote: > > I'm also wondering about the call to flush in run(PrintStream out, > > PrintStream err, String... args). It looks like the intention was > > to > > flush the wrapping PrintWriter. > >

Re: Faulty Null-Check Suspected in ToolProvider

On 2/16/19 12:20 AM, Philipp Kunz wrote: I'm also wondering about the call to flush in run(PrintStream out, PrintStream err, String... args). It looks like the intention was to flush the wrapping PrintWriter. That is not possible without also flushing the underlying PrintStream. BufferedWriter.

Re: Faulty Null-Check Suspected in ToolProvider

Hi again, I figured out and err deserve a null-check test as well. See patch. I'm wondering, if or how a similar check could be applied as well to ToolProvider.run(PrintWriter, PrintWriter, String...) which currently is implemented (hopefully) by each tool having to repeat the null-checks. I'm a

Re: Faulty Null-Check Suspected in ToolProvider

diff -r 7c17199fa37d src/java.base/share/classes/java/util/spi/ToolProvider.java --- a/src/java.base/share/classes/java/util/spi/ToolProvider.java Fri Feb 15 08:21:08 2019 -0500 +++ b/src/java.base/share/classes/java/util/spi/ToolProvider.java Sat Feb 16 00:57:54 2019 +0100 @@ -126,8 +126,9 @@

Re: Faulty Null-Check Suspected in ToolProvider

Hi Philipp This probably makes sense to update. Can you also update the ToolProviderTest.java to add a test for the changes Thank you Best Lance > On Feb 15, 2019, at 4:43 PM, Philipp Kunz wrote: > >

Faulty Null-Check Suspected in ToolProvider

Hi, There has always been an implicit null-check for args by the for loop attempting to iterate but the check seems to be intended and wrong for the elements. See attached patch. >From the comment to args, "the command-line arguments for the tool", I guess it makes sense to conclude that these sho