8025690: Default FileHandler constructor doesn't throw NullPointerException if pattern is empty and count > 1

[Lance @ Oracle]

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.Andersen at oracle.com
Sent from my iPad

> On Oct 1, 2014, at 6:00 AM, Daniel Fuchs <daniel.fuchs at 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.fuchs at oracle.com
>> <mailto:daniel.fuchs at 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.fuchs at oracle.com
>>>> <mailto:daniel.fuchs at 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.Andersen at oracle.com <mailto:Lance.Andersen at 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.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>> 
>> 
>> 
>