Hi Daniel, On Sep 30, 2014, at 5:20 PM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
> Hi Lance, > > Thanks for the comments! > > On 9/30/14 9:09 PM, Lance Andersen wrote: >> Hi Daniel, >> >> 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 > The only difference is that the default constructor reads the value > of 'pattern' from the configuration, whereas the other constructor get > the value supplied by the caller, > > So I am more curious about why the default constructor is not specified to > throw IllegalArgumentException if the pattern is empty - as all the other > constructor do... Possibly it's because the original author thought > that NPE would always be throw by openFile() if the pattern was empty. > > In this case I believe it's better to make the implementation > conform to the spec - even if the spec looks odd - rather than trying > to change the spec. > >> Just curious why you chose to put the check into openFiles() if the issue is >> just with the default constructor? > It was not possible to do the check in configure() since all the constructors > call configure - but only the default constructor use the actual value > set by configure() - the other constructors simply reset the value of > 'pattern' after configure() has been called, thus overriding the default > value read by the configuration. > > 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 Best Lance > > best regards, > > -- daniel >> >> Best, >> Lance >> On Sep 30, 2014, at 10:04 AM, Daniel Fuchs <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> >> >> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 >> Oracle Java Engineering >> 1 Network Drive >> Burlington, MA 01803 >> lance.ander...@oracle.com >> >> >> > Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
