harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib] BufferedReader and FileInputStream.available()
Date Fri, 06 Nov 2009 10:18:06 GMT
On 06/Nov/2009 03:06, Regis wrote:
> Mark Hindess wrote:
>> In message <96933a4d0911051045p61431af5ie2cecb850a62267a@mail.gmail.com>,
>> enh writes:
>>> On Wed, Nov 4, 2009 at 23:07, Regis <xu.regis@gmail.com> wrote:
>>>> Mark Hindess wrote:
>>>>> In message <4AF0DC18.1000504@gmail.com>, Regis writes:
>>>>>
>>>>> Regis, these are really two different issues so lets treat them as
>>>>> such.  I actually think fixing the /proc reading is better done
>>>>> in the java code.  I've attached a patch that just catches and
>>>>> ignores the IOException from the available call.  I think this is
>>>>> reasonable since if the exception was "real" then the read will
>>>>> fail if a similar exception.  In fact, since the user is doing
>>>>> a read call, having the exception thrown by the read is more in
>>>>> keeping with the expectations of the user than having an exception
>>>>> thrown by the available call.
>>>>
>>>> Thanks Mark, I ran luni tests with your patch, no regression found,
>>>> I'll leave patch to you to apply :)
>>>>
>>>> invoking in.available before read is a little overhead, in the
>>>> worst case, one read need four system calls, three seek and one
>>>> read. Actually the condition
>>>>
>>>> if ((in.available() = 0) && (out.position() > offset))
>>>>
>>>> it's only for tty file, for normal file it's not necessary. It's
>>>> better if we can remove the check for normal file, but I have no
>>>> idea how to do this.
>>> isatty(3). for the price of one JNI call at construction and a boolean
>>> field to record the result, you could avoid any later calls.
>>
>> This might be a good idea, but I'm getting confused...
>>
>> At the moment there seems to be a special case in
>> FileInputStream.available()
>> like this:
>>
>>   // stdin requires special handling
>>   if (fd == FileDescriptor.in) {
>>     return (int) fileSystem.ttyAvailable();
>>   }
>>   ... perform seek based available check ...
>>
>> but I find this confusing because the comment and check implies that
>> stdin is special but the fileSystem method name implies that it is being
>> a tty that is the distinction.  This is confusing because *any* file
>> descriptor can be a tty - e.g. new FileReader("/dev/tty") - and stdin
>> doesn't have to be a tty - "java </dev/zero".
> 
> Good question, I suppose here should be like this:
> if (istty(fd)) {
>   return (int) fileSystem.ttyAvailable(fd);
> }

With apologies for being pedantic, but IMHO it should be fd.istty()
since there are likely a few places where this could be useful, and we
should cache it in the right place.

Regards,
Tim

>> Regis, can you explain a little more about why the check is needed
>> in the available call?  And why the available check is needed in the
>> FileInputStream read method?
> 
> After I removed the check, everything is fine, except following test is
> failed:
> 
>   public static void main(String args[])
>   {
>     BufferedReader reader = new BufferedReader(new
> InputStreamReader(System.in));
>     String line = null;
>     try {
>       line = reader.readLine();
>     } catch (IOException e) {
>       e.printStackTrace();
>     }
>     System.out.println(line);
>   }
> 
>>
>> In each case, is it really stdin that is special or any file descriptor
>> representing a tty?
>>
>> I'd like to understand this as there are a whole bunch of ttyRead with
>> a similar check that only affects stdin not any tty descriptor.
>>
>>> in the meantime, i wondered also whether it's worth swapping the two
>>> conjuncts in the if, since the latter is essentially free while the
>>> former is expensive. (i've no idea how often the cheap conjunct is
>>> false, though.)
>>
>> I thought about this but held of doing it for the same reason.  However,
>> I guess it is almost certainly a win.
>>
>> Regards,
>>  Mark.
>>
>>
>>
> 
> 

Mime
View raw message