harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: svn commit: r773966 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSFileSystem.java
Date Wed, 13 May 2009 10:43:56 GMT
sebb wrote:
> On 13/05/2009, Oliver Deakin <oliver.deakin@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,  <odeakin@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


Mime
View raw message