[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] request for review: 8074954: ImageInputStreamImpl.readShort/readInt do not
From: Sergey Bylokhov <Sergey.Bylokhov () oracle ! com>
Date: 2015-03-20 18:03:45
Message-ID: 550C6101.6090008 () oracle ! com
[Download RAW message or body]
Right. It will be thrown on the next iteration.
The fix looks fine then.
20.03.15 20:56, Phil Race wrote:
> I see. You were referring to the implementation. But your snippet
> doesn't show the loop which should handle this case properly :
> 350 while (len > 0) {
> 351 int nbytes = read(b, off, len);
> 352 if (nbytes == -1) {
> 353 throw new EOFException();
> 354 }
> 355 off += nbytes;
> 356 len -= nbytes;
> 357 }
>
> -phil.
>
> On 03/20/2015 10:54 AM, Sergey Bylokhov wrote:
>>> Andrew, The fix looks good to me.
>>>
>>> Sergey, are you asking for that text to be added ? Looks to me like
>>> its already there,
>>> so probably I am not following what you mean.
>> No, readFully() has this code:
>> int nbytes = read(b, off, len);
>> if (nbytes == -1) {
>> throw new EOFException();
>> }
>> I assume this method should throw an exception if nbytes!=len as
>> described in the specification. same as readShort/readInt.
>>
>>>
>>> -phil.
>>>
>>> On 03/20/2015 10:32 AM, Sergey Bylokhov wrote:
>>>> Hi, Andrew?
>>>> Hi, Andrew.
>>>> Should we update a readFully() also?
>>>> * @exception java.io.EOFException if the stream reaches the
>>>> end before
>>>> * reading all the bytes.
>>>> * @exception IOException if an I/O error occurs.
>>>> */
>>>> void readFully(byte[] b, int off, int len) throws IOException;
>>>>
>>>> 20.03.15 14:30, Andrew Brygin wrote:
>>>>> Hello,
>>>>>
>>>>> could you please review a fix for CR 8074954?
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8074954
>>>>> Webrev: http://cr.openjdk.java.net/~bae/8074954/9/webrev.00/
>>>>>
>>>>> The problem happens if an input stream does not contain enough data
>>>>> to read a multi-byte type (as 4-bytes integer or 2-bytes short)
>>>>> completely. In this case the actual number of obtained bytes is
>>>>> returned, and if we get at least one byte, the EOF exception in
>>>>> not triggered.
>>>>> As a result, an incorrect value is returned.
>>>>>
>>>>> Suggested fix is to check explicitly whether required number of
>>>>> bytes
>>>>> has been read.
>>>>>
>>>>> Supplied regression test demonstrates the problem.
>>>>>
>>>>> Please take a look.
>>>>>
>>>>> Thanks,
>>>>> Andrew.
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>>
>>
>>
>> --
>> Best regards, Sergey.
>
--
Best regards, Sergey.
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Right. It will be thrown on the next
iteration.<br>
The fix looks fine then.<br>
<br>
20.03.15 20:56, Phil Race wrote:<br>
</div>
<blockquote cite="mid:550C5F60.5040703@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">I see. You were referring to the
implementation. But your snippet<br>
doesn't show the loop which should handle this case properly :<br>
<pre> 350 while (len > 0) {
351 int nbytes = read(b, off, len);
352 if (nbytes == -1) {
353 throw new EOFException();
354 }
355 off += nbytes;
356 len -= nbytes;
357 }</pre>
<br>
-phil.<br>
<br>
On 03/20/2015 10:54 AM, Sergey Bylokhov wrote:<br>
</div>
<blockquote cite="mid:550C5ED7.4060608@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<blockquote cite="mid:550C5CA6.2070401@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Andrew, The fix looks good to me.<br>
<br>
Sergey, are you asking for that text to be added ? Looks to
me like its already there,<br>
so probably I am not following what you mean.<br>
</div>
</blockquote>
No, readFully() has this code:<br>
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
int nbytes = read(b, off, len);<br>
if (nbytes == -1) {<br>
throw new EOFException();<br>
}<br>
I assume this method should throw an exception if nbytes!=len as
described in the specification. same as
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
readShort/readInt.<br>
<br>
<blockquote cite="mid:550C5CA6.2070401@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
-phil.<br>
<br>
On 03/20/2015 10:32 AM, Sergey Bylokhov wrote:<br>
</div>
<blockquote cite="mid:550C59C4.6000000@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi, Andrew?<br>
Hi, Andrew.<br>
Should we update a readFully() also?<br>
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
* @exception java.io.EOFException if the stream
reaches the end before<br>
* reading all the bytes.<br>
* @exception IOException if an I/O error occurs.<br>
*/<br>
void readFully(byte[] b, int off, int len) throws
IOException;<br>
<br>
20.03.15 14:30, Andrew Brygin wrote:<br>
</div>
<blockquote cite="mid:550C04E9.2070807@oracle.com"
type="cite">Hello, <br>
<br>
could you please review a fix for CR 8074954? <br>
<br>
Bug: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8074954">https://bugs.openjdk.java.net/browse/JDK-8074954</a>
<br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ebae/8074954/9/webrev.00/">http://cr.openjdk.java.net/~bae/8074954/9/webrev.00/</a>
<br>
<br>
The problem happens if an input stream does not contain
enough data <br>
to read a multi-byte type (as 4-bytes integer or 2-bytes
short) <br>
completely. In this case the actual number of obtained
bytes is <br>
returned, and if we get at least one byte, the EOF
exception in <br>
not triggered. <br>
As a result, an incorrect value is returned. <br>
<br>
Suggested fix is to check explicitly whether required
number of bytes <br>
has been read. <br>
<br>
Supplied regression test demonstrates the problem. <br>
<br>
Please take a look. <br>
<br>
Thanks, <br>
Andrew. <br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Best regards, Sergey. </pre>
</blockquote>
<br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Best regards, Sergey. </pre>
</blockquote>
<br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Best regards, Sergey. </pre>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic