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 Mon, 18 May 2009 17:17:38 GMT
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 <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