You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Regis <xu...@gmail.com> on 2009/11/25 10:15:14 UTC

Re: [testing] review request

Tim Ellison wrote:
> Could somebody please review the patch on HARMONY-6386 and approve it's
> application for M12?
> 
> After the milestone I'll refactor the code to move this utility method
> into LUNI.  For now this is minimal disruption.
> 
> Regards,
> Tim
> 

All sound tests are passed after the patch. I think 
getAllBytesFromStreamAndClose can be simplified, but maybe do it after M12.

> On 25/Nov/2009 02:53, Regis wrote:
>> Jesse Wilson wrote:
>>> On Tue, Nov 24, 2009 at 9:12 AM, Tim Ellison <t....@gmail.com>
>>> wrote:
>>>
>>>> Seems to be caused by a change in the implementation of available() for
>>>> InputStreams on Zip entries.
>>>>
>>>> The failing code is in
>>>> org.apache.harmony.sound.utils.ProviderService#getProviders(String) when
>>>> it tries to read the whole content using available()...
>>>>
>>>> <snip>
>>>>   InputStream in = urls.nextElement()
>>>>           .openStream();
>>>>   bytes = new byte[in.available()];
>>>>   in.read(bytes);
>>>>   in.close();
>>>> <snip>
>>>>
>>> Aahhhh.. gotcha.
>>>
>>>
>>>
>>>> Of course, this is bad form, and when I switch to use the old favourite
>>>> getAllBytesFromStreamAndClose(InputStream) things start working again.
>>>>
>>> Sweet.
>>>
>>>
>>>> We can debate whether we should try to improve the available() impl
>>>> or not.
>>>>
>>> If we want to be true to the
>>> spec<http://java.sun.com/javase/6/docs/api/java/util/zip/InflaterInputStream.html#available()>,
>>>
>>> we can't change it.
>>>
>>>
>>> public int *available*()
>>>               throws IOException <../../../java/io/IOException.html>
>>>
>>> Returns 0 after EOF has been reached, otherwise always return 1.
>>>
>>> Programs should not count on this method to return the actual number of
>>> bytes that could be read without blocking.
>> Agree. Changing to use loop to read data, all sound tests can pass.
>>
>>> *Overrides:*available
>>> <../../../java/io/FilterInputStream.html#available()> in
>>> class FilterInputStream <../../../java/io/FilterInputStream.html>
>>> *Returns:*1
>>> before EOF and 0 after
>>> EOF.*Throws:*IOException<../../../java/io/IOException.html> -
>>> if an I/O error occurs.
>>>
>>
> 


-- 
Best Regards,
Regis.

Re: [testing] review request

Posted by Tim Ellison <t....@gmail.com>.
On 25/Nov/2009 09:15, Regis wrote:
> Tim Ellison wrote:
>> Could somebody please review the patch on HARMONY-6386 and approve it's
>> application for M12?
>>
>> After the milestone I'll refactor the code to move this utility method
>> into LUNI.  For now this is minimal disruption.
>>
>> Regards,
>> Tim
>>
> 
> All sound tests are passed after the patch. I think
> getAllBytesFromStreamAndClose can be simplified, but maybe do it after M12.

Great.  If you want to get your ideas down please send them here, even
if we don't commit them until after the milestone.

Regards,
Tim