db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-3941) Unsafe use of DataInput.skipBytes() in StoredPage and StoredFieldHeader
Date Sun, 12 Apr 2009 16:38:14 GMT

    [ https://issues.apache.org/jira/browse/DERBY-3941?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12698237#action_12698237
] 

Knut Anders Hatlen commented on DERBY-3941:
-------------------------------------------

Hi Yun,

My point about DataInput.readByte() was that -1 is not an indication that the end of the stream
has been reached, so it's not correct to throw an EOFException when readByte() returns -1.
I think skipFully() will have to be implemented along the lines of this untested code:

    public static void skipFully(DataInput in, int bytesToSkip)
        throws IOException
    {
        if (in == null) {
            throw new NullPointerException();
        }

        while (bytesToSkip > 0) {
            int skipped = in.skipBytes(bytesToSkip);
            if (skipped == 0) {
                // No bytes skipped, read one byte to see if EOF has been
                // reached. DataInput.readByte() will throw an EOFException
                // if there's nothing more to read.
                in.readByte();
                // Still more data to read. Account for the byte we just read.
                skipped++;
            } 
            bytesToSkip -= skipped;
       }
    }

The changes to the javadoc for skipPersistent(InputStream,int) also look wrong, as that method
is not supposed to throw an EOFException if end of stream is reached before all the bytes
have been skipped. If it reaches end of stream prematurely, it returns the number of bytes
actually skipped, just as the old javadoc said.

As to the compile errors when using the same name for the different variants of skipFully(),
it should be possible to disambiguate the method calls by using a cast, like this:
    InputStreamUtil.skipFully((InputStream) null, x);
or
    InputStreamUtil.skipFully((DataInput) null, x);

But perhaps the problem here is that we're trying to overload InputStreamUtil too much. Since
a DataInput is not (necessarily) an InputStream it might be cleaner to create a new class,
DataInputUtil, where we can collect utility methods for DataInputs without conflicting with
the utilities for InputStreams.

> Unsafe use of DataInput.skipBytes() in StoredPage and StoredFieldHeader
> -----------------------------------------------------------------------
>
>                 Key: DERBY-3941
>                 URL: https://issues.apache.org/jira/browse/DERBY-3941
>             Project: Derby
>          Issue Type: Bug
>          Components: Newcomer, Store
>            Reporter: Knut Anders Hatlen
>            Assignee: Yun Lee
>            Priority: Minor
>         Attachments: derby-3941-1.diff, derby-3941-1.stat, derby-3941-2.diff, derby-3941-2.stat
>
>
> Some methods in StoredFileHeader and StoredPage call java.io.DataInput.skipBytes(int)
with the assumption that it always skips the requested number of bytes. According to the javadoc
for skipBytes, it may skip fewer bytes than requested, possibly 0, even if the end of the
stream hasn't been reached.
> The problem exists in these methods:
>   StoredFieldHeader.readFieldDataLength()
>   StoredPage.readRecordFromStream()
>   StoredPage.skipField()
>   StoredPage.readOneColumnFromPage()
>   StoredPage.readRecordFromArray()
> We should change the code so that it works correctly even if skipBytes() were to skip
fewer bytes than requested.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message