You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Nathan Beyer <nd...@apache.org> on 2009/05/13 03:15:25 UTC
Re: svn commit: r773966 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
This commit seems contradictory to the specification. All JREs must
support UTF-8 as an encoding, so the UnsupportedEncodingException
should never happen.
I think the code should throw new AssertionError(e) in the catch block.
Thoughts?
-Nathan
On Tue, May 12, 2009 at 11:23 AM, <od...@apache.org> wrote:
> Author: odeakin
> Date: Tue May 12 16:23:08 2009
> New Revision: 773966
>
> URL: http://svn.apache.org/viewvc?rev=773966&view=rev
> Log:
> When constructing the exception, make sure the error message is in an encoding where it will be readable on non-ASCII platforms once printed.
>
> Modified:
> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>
> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java?rev=773966&r1=773965&r2=773966&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java Tue May 12 16:23:08 2009
> @@ -20,6 +20,7 @@
> import java.io.FileDescriptor;
> import java.io.FileNotFoundException;
> import java.io.IOException;
> +import java.io.UnsupportedEncodingException;
>
> /**
> * This is the portable implementation of the file system interface.
> @@ -235,7 +236,11 @@
> }
> long handler = openImpl(fileName, mode);
> if (handler < 0) {
> - throw new FileNotFoundException(new String(fileName));
> + try {
> + throw new FileNotFoundException(new String(fileName, "UTF-8"));
> + } catch (java.io.UnsupportedEncodingException e) {
> + throw new FileNotFoundException(new String(fileName));
> + }
> }
> return handler;
> }
>
>
>
Re: svn commit: r773966 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
Posted by Oliver Deakin <ol...@googlemail.com>.
I have committed the change diff'ed below - I think it satisfies all the
opinions here and is the best method of reporting the exceptions that
have occurred.
Regards,
Oliver
Oliver Deakin wrote:
> sebb wrote:
>> On 13/05/2009, Oliver Deakin <ol...@googlemail.com> wrote:
>>
>>> Nathan Beyer wrote:
>>>
>>>
>>>> This commit seems contradictory to the specification. All JREs must
>>>> support UTF-8 as an encoding, so the UnsupportedEncodingException
>>>> should never happen.
>>>>
>>>> I think the code should throw new AssertionError(e) in the catch
>>>> block.
>>>>
>>>>
>>>>
>>> I'm not sure I'd call it contradictory to the spec - the exception
>>> needs to
>>> be caught as it is declared by the String constructor being called,
>>> whether
>>> it should happen or not.
>>>
>>
>> +1
>>
>>
>>> How it is handled is up to us - perhaps an
>>> AssertionError would be better here to indicate that we have entered
>>> a code
>>> block that should be unreachable. I don't feel strongly either way
>>> so am
>>> happy to go with whatever the consensus feeling on this is.
>>>
>>
>> Ideally, it would be nice if you could report both the failure to open
>> the file, and the failure to convert the filename using UTF-8.
>>
>> The UEE should be impossible, but perhaps you can include the FNF
>> details in the AssertionError somehow?
>>
>
> Agreed - I originally left the creation of the FileNotFoundException
> so that the original exception would still be presented to the user,
> but I would prefer if we could throw an AssertionError caused by the
> UEE caused by the FNF.
>
> Perhaps something along the lines of:
>
> try {
> throw new FileNotFoundException(new
> String(fileName, "UTF-8"));
> } catch (java.io.UnsupportedEncodingException e) {
> - throw new FileNotFoundException(new
> String(fileName));
> + // UTF-8 should always be supported, so throw
> an assertion
> + FileNotFoundException fnfe = new
> FileNotFoundException(new String(fileName));
> + e.initCause(fnfe);
> + throw new AssertionError(e);
> }
> }
> return handler;
>
> Regards,
> Oliver
>
>> Just my 2p.
>>
>>
>>> Regards,
>>> Oliver
>>>
>>>
>>>
>>>
>>>> Thoughts?
>>>>
>>>> -Nathan
>>>>
>>>> On Tue, May 12, 2009 at 11:23 AM, <od...@apache.org> wrote:
>>>>
>>>>
>>>>
>>>>> Author: odeakin
>>>>> Date: Tue May 12 16:23:08 2009
>>>>> New Revision: 773966
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=773966&view=rev
>>>>> Log:
>>>>> When constructing the exception, make sure the error message is in an
>>>>>
>>> encoding where it will be readable on non-ASCII platforms once printed.
>>>
>>>>> Modified:
>>>>>
>>>>>
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>>
>>>
>>>>> Modified:
>>>>>
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>>
>>>
>>>>> URL:
>>>>>
>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java?rev=773966&r1=773965&r2=773966&view=diff
>>>
>>>
>>> ==============================================================================
>>>
>>>
>>>>> ---
>>>>>
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>>
>>> (original)
>>>
>>>>> +++
>>>>>
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>>
>>> Tue May 12 16:23:08 2009
>>>
>>>>> @@ -20,6 +20,7 @@
>>>>> import java.io.FileDescriptor;
>>>>> import java.io.FileNotFoundException;
>>>>> import java.io.IOException;
>>>>> +import java.io.UnsupportedEncodingException;
>>>>>
>>>>> /**
>>>>> * This is the portable implementation of the file system interface.
>>>>> @@ -235,7 +236,11 @@
>>>>> }
>>>>> long handler = openImpl(fileName, mode);
>>>>> if (handler < 0) {
>>>>> - throw new FileNotFoundException(new
>>>>>
>>> String(fileName));
>>>
>>>>> + try {
>>>>> + throw new FileNotFoundException(new
>>>>>
>>> String(fileName, "UTF-8"));
>>>
>>>>> + } catch
>>>>>
>>> (java.io.UnsupportedEncodingException e) {
>>>
>>>>> + throw new FileNotFoundException(new
>>>>>
>>> String(fileName));
>>>
>>>>> + }
>>>>> }
>>>>> return handler;
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>> --
>>> Oliver Deakin
>>> Unless stated otherwise above:
>>> IBM United Kingdom Limited - Registered in England and Wales with
>>> number
>>> 741598. Registered office: PO Box 41, North Harbour, Portsmouth,
>>> Hampshire
>>> PO6 3AU
>>>
>>>
>>>
>>
>>
>
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: svn commit: r773966 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
Posted by Oliver Deakin <ol...@googlemail.com>.
sebb wrote:
> On 13/05/2009, Oliver Deakin <ol...@googlemail.com> wrote:
>
>> Nathan Beyer wrote:
>>
>>
>>> This commit seems contradictory to the specification. All JREs must
>>> support UTF-8 as an encoding, so the UnsupportedEncodingException
>>> should never happen.
>>>
>>> I think the code should throw new AssertionError(e) in the catch block.
>>>
>>>
>>>
>> I'm not sure I'd call it contradictory to the spec - the exception needs to
>> be caught as it is declared by the String constructor being called, whether
>> it should happen or not.
>>
>
> +1
>
>
>> How it is handled is up to us - perhaps an
>> AssertionError would be better here to indicate that we have entered a code
>> block that should be unreachable. I don't feel strongly either way so am
>> happy to go with whatever the consensus feeling on this is.
>>
>
> Ideally, it would be nice if you could report both the failure to open
> the file, and the failure to convert the filename using UTF-8.
>
> The UEE should be impossible, but perhaps you can include the FNF
> details in the AssertionError somehow?
>
Agreed - I originally left the creation of the FileNotFoundException so
that the original exception would still be presented to the user, but I
would prefer if we could throw an AssertionError caused by the UEE
caused by the FNF.
Perhaps something along the lines of:
try {
throw new FileNotFoundException(new
String(fileName, "UTF-8"));
} catch (java.io.UnsupportedEncodingException e) {
- throw new FileNotFoundException(new
String(fileName));
+ // UTF-8 should always be supported, so throw
an assertion
+ FileNotFoundException fnfe = new
FileNotFoundException(new String(fileName));
+ e.initCause(fnfe);
+ throw new AssertionError(e);
}
}
return handler;
Regards,
Oliver
> Just my 2p.
>
>
>> Regards,
>> Oliver
>>
>>
>>
>>
>>> Thoughts?
>>>
>>> -Nathan
>>>
>>> On Tue, May 12, 2009 at 11:23 AM, <od...@apache.org> wrote:
>>>
>>>
>>>
>>>> Author: odeakin
>>>> Date: Tue May 12 16:23:08 2009
>>>> New Revision: 773966
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=773966&view=rev
>>>> Log:
>>>> When constructing the exception, make sure the error message is in an
>>>>
>> encoding where it will be readable on non-ASCII platforms once printed.
>>
>>>> Modified:
>>>>
>>>>
>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>
>>>> Modified:
>>>>
>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>
>>>> URL:
>>>>
>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java?rev=773966&r1=773965&r2=773966&view=diff
>>
>> ==============================================================================
>>
>>>> ---
>>>>
>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>> (original)
>>
>>>> +++
>>>>
>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>> Tue May 12 16:23:08 2009
>>
>>>> @@ -20,6 +20,7 @@
>>>> import java.io.FileDescriptor;
>>>> import java.io.FileNotFoundException;
>>>> import java.io.IOException;
>>>> +import java.io.UnsupportedEncodingException;
>>>>
>>>> /**
>>>> * This is the portable implementation of the file system interface.
>>>> @@ -235,7 +236,11 @@
>>>> }
>>>> long handler = openImpl(fileName, mode);
>>>> if (handler < 0) {
>>>> - throw new FileNotFoundException(new
>>>>
>> String(fileName));
>>
>>>> + try {
>>>> + throw new FileNotFoundException(new
>>>>
>> String(fileName, "UTF-8"));
>>
>>>> + } catch
>>>>
>> (java.io.UnsupportedEncodingException e) {
>>
>>>> + throw new FileNotFoundException(new
>>>>
>> String(fileName));
>>
>>>> + }
>>>> }
>>>> return handler;
>>>> }
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>> --
>> Oliver Deakin
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with number
>> 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
>> PO6 3AU
>>
>>
>>
>
>
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: svn commit: r773966 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
Posted by sebb <se...@gmail.com>.
On 13/05/2009, Oliver Deakin <ol...@googlemail.com> wrote:
>
> Nathan Beyer wrote:
>
> > This commit seems contradictory to the specification. All JREs must
> > support UTF-8 as an encoding, so the UnsupportedEncodingException
> > should never happen.
> >
> > I think the code should throw new AssertionError(e) in the catch block.
> >
> >
>
> I'm not sure I'd call it contradictory to the spec - the exception needs to
> be caught as it is declared by the String constructor being called, whether
> it should happen or not.
+1
> How it is handled is up to us - perhaps an
> AssertionError would be better here to indicate that we have entered a code
> block that should be unreachable. I don't feel strongly either way so am
> happy to go with whatever the consensus feeling on this is.
Ideally, it would be nice if you could report both the failure to open
the file, and the failure to convert the filename using UTF-8.
The UEE should be impossible, but perhaps you can include the FNF
details in the AssertionError somehow?
Just my 2p.
> Regards,
> Oliver
>
>
>
> > Thoughts?
> >
> > -Nathan
> >
> > On Tue, May 12, 2009 at 11:23 AM, <od...@apache.org> wrote:
> >
> >
> > > Author: odeakin
> > > Date: Tue May 12 16:23:08 2009
> > > New Revision: 773966
> > >
> > > URL: http://svn.apache.org/viewvc?rev=773966&view=rev
> > > Log:
> > > When constructing the exception, make sure the error message is in an
> encoding where it will be readable on non-ASCII platforms once printed.
> > >
> > > Modified:
> > >
> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
> > >
> > > Modified:
> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
> > > URL:
> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java?rev=773966&r1=773965&r2=773966&view=diff
> > >
> ==============================================================================
> > > ---
> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
> (original)
> > > +++
> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
> Tue May 12 16:23:08 2009
> > > @@ -20,6 +20,7 @@
> > > import java.io.FileDescriptor;
> > > import java.io.FileNotFoundException;
> > > import java.io.IOException;
> > > +import java.io.UnsupportedEncodingException;
> > >
> > > /**
> > > * This is the portable implementation of the file system interface.
> > > @@ -235,7 +236,11 @@
> > > }
> > > long handler = openImpl(fileName, mode);
> > > if (handler < 0) {
> > > - throw new FileNotFoundException(new
> String(fileName));
> > > + try {
> > > + throw new FileNotFoundException(new
> String(fileName, "UTF-8"));
> > > + } catch
> (java.io.UnsupportedEncodingException e) {
> > > + throw new FileNotFoundException(new
> String(fileName));
> > > + }
> > > }
> > > return handler;
> > > }
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
>
> --
> Oliver Deakin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
> PO6 3AU
>
>
Re: svn commit: r773966 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
Posted by Oliver Deakin <ol...@googlemail.com>.
Nathan Beyer wrote:
> This commit seems contradictory to the specification. All JREs must
> support UTF-8 as an encoding, so the UnsupportedEncodingException
> should never happen.
>
> I think the code should throw new AssertionError(e) in the catch block.
>
I'm not sure I'd call it contradictory to the spec - the exception needs
to be caught as it is declared by the String constructor being called,
whether it should happen or not. How it is handled is up to us -
perhaps an AssertionError would be better here to indicate that we have
entered a code block that should be unreachable. I don't feel strongly
either way so am happy to go with whatever the consensus feeling on this is.
Regards,
Oliver
> Thoughts?
>
> -Nathan
>
> On Tue, May 12, 2009 at 11:23 AM, <od...@apache.org> wrote:
>
>> Author: odeakin
>> Date: Tue May 12 16:23:08 2009
>> New Revision: 773966
>>
>> URL: http://svn.apache.org/viewvc?rev=773966&view=rev
>> Log:
>> When constructing the exception, make sure the error message is in an encoding where it will be readable on non-ASCII platforms once printed.
>>
>> Modified:
>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java?rev=773966&r1=773965&r2=773966&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java Tue May 12 16:23:08 2009
>> @@ -20,6 +20,7 @@
>> import java.io.FileDescriptor;
>> import java.io.FileNotFoundException;
>> import java.io.IOException;
>> +import java.io.UnsupportedEncodingException;
>>
>> /**
>> * This is the portable implementation of the file system interface.
>> @@ -235,7 +236,11 @@
>> }
>> long handler = openImpl(fileName, mode);
>> if (handler < 0) {
>> - throw new FileNotFoundException(new String(fileName));
>> + try {
>> + throw new FileNotFoundException(new String(fileName, "UTF-8"));
>> + } catch (java.io.UnsupportedEncodingException e) {
>> + throw new FileNotFoundException(new String(fileName));
>> + }
>> }
>> return handler;
>> }
>>
>>
>>
>>
>
>
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU