hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yi Liu (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HADOOP-10628) Javadoc and few code style improvement for Crypto input and output streams
Date Sun, 25 May 2014 13:15:04 GMT

     [ https://issues.apache.org/jira/browse/HADOOP-10628?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel

Yi Liu updated HADOOP-10628:

    Attachment: HADOOP-10628.patch

Thanks [~clamb] for the comments. The patch includes update for these comments.

Response to some comments:
Do you have a test that verifies that seek with an exception unwinds
the seek position correctly? The contract is to not move the position
if an exception occurs.
OK, let's add position check after seek with exception.

Why create curOffset at all? It looks like it is only used to pass to
resetStreamOffset below.

{{curOffset}} is to cache {{streamOffset}}, since {{streamOffset}} will be changed after 
the first {{resetStreamOffSet(position)}}

+ // If target pos we have already read and decrypt.

This comment is confusing. Could you please reword it?

For the code above, I wonder if we shouldn't maintain a reference to
the actual ByteBuffer once it is known to be ByteBufferReadable. If
the caller switches BBs, then it is possible that this could throw a
UnsupportedOperationException. So the check would be to see if the BB
was the same one that was already known to be BBReadable, and if not,
then check it again.
{{ByteBufferReadable}} is decided by the underlying steam when {{CryptoInputStream}} is created,
and the {{inBuffer}} is owned by {{CryptoInputStream}}, the caller may not switch the underlying
stream or the {{inBuffer}}.

+ @Override
+ public int read(ByteBuffer buf) throws IOException {
+ checkStream();
+ if (in instanceof ByteBufferReadable) {

It would be good if we didn't have to do this instanceof check here.
It's necessary. We can also see this in {{FSDataInputStream}}

> Javadoc and few code style improvement for Crypto input and output streams
> --------------------------------------------------------------------------
>                 Key: HADOOP-10628
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10628
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: security
>    Affects Versions: fs-encryption (HADOOP-10150 and HDFS-6134)
>            Reporter: Yi Liu
>            Assignee: Yi Liu
>             Fix For: fs-encryption (HADOOP-10150 and HDFS-6134)
>         Attachments: HADOOP-10628.patch
> There are some additional comments from [~clamb] related to javadoc and few code style
on HADOOP-10603, let's fix them in this follow-on JIRA.

This message was sent by Atlassian JIRA

View raw message