[
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:
{quote}
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.
{quote}
OK, let's add position check after seek with exception.
{quote}
Why create curOffset at all? It looks like it is only used to pass to
resetStreamOffset below.
{quote}
{{curOffset}} is to cache {{streamOffset}}, since {{streamOffset}} will be
changed after
the first {{resetStreamOffSet(position)}}
{quote}
+ // If target pos we have already read and decrypt.
This comment is confusing. Could you please reword it?
{quote}
Sure.
{quote}
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.
{quote}
{{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}}.
{quote}
+ @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.
{quote}
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
(v6.2#6252)
