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

[Daniel Fuchs]

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>
>
>
>