Hi Daniel, The revised changes look fine.
Best, Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPad > On Oct 1, 2014, at 6:00 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > Hi Lance, > >> On 30/09/14 23:55, Lance Andersen wrote: >> On Sep 30, 2014, at 5:20 PM, Daniel Fuchs <daniel.fu...@oracle.com >> <mailto:daniel.fu...@oracle.com>> wrote: >>> On 9/30/14 9:09 PM, Lance Andersen wrote: >>>> Shouldn't the other constructors indicate that an NPE will occur >>>> similar to the default constructor if the pattern is null? >>> >>> I assume that for the other constructors the NPE falls in the category >>> "parameters should not be null unless otherwise specified" which is >>> the usual rule for java.util.logging. >> >> I have gotten mixed views on this and have gone and clarified this in >> some cases. Just think consistency would be good but your call > > Thanks. I'd prefer not to modify the API documentation in this patch, > just to make it clear that the patch does not modify the spec. > There's some text somewhere in java.util.logging javadoc - either > package description or LogManager description that explicitely says > that unless otherwise specified null parameters will trigger NPE. > I can't say I like it very much but in the present case I think > I'm going to take advantage of it ;-) . > > [...] > >>>> Just curious why you chose to put the check into openFiles() if the >>>> issue is just with the default constructor? > [...] >>> I could have placed the check in the default constructor between >>> the call to configure() and the call to openFiles() - but I thought >>> it was better to put it in openFiles() - since the spirit of the spec >>> seems to be that pattern should be neither null nor empty >>> when openFiles() is called. >> >> Guess the question is whether you feel the check is redundant for the >> other code paths to openFiles(). Your call either way as the change >> seems fine otherwise > > It's a good remark. I reworked the patch to do the check in the > default constructor, and put an assert in openFiles(). > It's much clearer this way. > > http://cr.openjdk.java.net/~dfuchs/webrev_8025690/webrev.01 > > Thanks Lance! > > -- daniel > >> >> Best >> Lance >>> >>> best regards, >>> >>> -- daniel >>>> >>>> Best, >>>> Lance >>>> On Sep 30, 2014, at 10:04 AM, Daniel Fuchs <daniel.fu...@oracle.com >>>> <mailto:daniel.fu...@oracle.com>> wrote: >>>> >>>>> Hi, >>>>> >>>>> Please find below a fix for >>>>> >>>>> 8025690: Default FileHandler constructor doesn't >>>>> throw NullPointerException if pattern is empty and count > 1 >>>>> https://bugs.openjdk.java.net/browse/JDK-8025690 >>>>> >>>>> The default constructor of FileHandler is specified to throw >>>>> a NullPointerException if the pattern property string is an >>>>> empty string. >>>>> (see >>>>> <http://docs.oracle.com/javase/8/docs/api/java/util/logging/FileHandler.html#FileHandler-->) >>>>> >>>>> However it strangely does so only when count=1 >>>>> >>>>> The fix adds an additional check in openFiles() to verify that >>>>> the pattern is not empty. At this point the other constructors >>>>> (which take a pattern as parameter) will already have thrown an >>>>> IAE if the pattern was empty (or an NPE if it was null). >>>>> >>>>> http://cr.openjdk.java.net/~dfuchs/webrev_8025690/webrev.00 >>>>> >>>>> best regards, >>>>> >>>>> -- daniel >>>> >>>> <Mail Attachment.gif> >>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> >>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance >>>> Andersen| Principal Member of Technical Staff | +1.781.442.2037 >>>> Oracle Java Engineering >>>> 1 Network Drive >>>> Burlington, MA 01803 >>>> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> >>>> >>>> >>>> >>> >> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> >> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| >> Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com> >> >> >> >
