[
https://issues.apache.org/jira/browse/DERBY-2379?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anurag Shekhar updated DERBY-2379:
----------------------------------
Attachment: derby-2379v3.diff
about derby-2379v3.diff
>The implementations of StorageRandomAccessFile normally extend
>java.io.RandomAccessFile which implements all the methods of the
>StorageRandomAccessFile interface. Could that help LOBFile as well?
Problem with extending RandomAccessFile is that I will need to intercept all
the methods to either throw not supported exception or to include encryption.
> * LOBFile.getBlocks:
> Is it necessary to check whether len is negative and thrown an
> IndexOutOfBoundsException? I guess the boundaries are checked on a
> higher level as well, and you'll get a NegativeArraySizeException
> anyway if len in negative, so I don't see any need to explicitly
> throw IndexOutOfBoundsException.
>Not addressed as far as I can see.
i have this checking beacuase stream classes need to throw
IndexOutOfBoundsException in case of negative length.
> * The read*/write* methods of LOBFile have many calls to
> RandomAccessFile.length(). Would it be possible to reduce the number
> of times it is called? I'm not sure, but I suspect length() might
> involve a system call, so one shouldn't call it too frequently. I
> was thinking perhaps we could change code that looks like this
> + if (currentPos >= randomAccessFile.length()) {
> + //current postion is in memory
> + int pos = (int) (currentPos - randomAccessFile.length());
> into something like this:
> long fileLength = randomAccessFile.length();
> if (currentPos >= fileLength) {
> int pos = (int) (currentPos - fileLength);
>Partly addressed. Still occurrences of multiple calls to length() in
>writeEncrypted(byte), writeEncrypted(byte[],int,int) and setLength().
fixed
> * LOBFile.readEncrypted(byte[],int,int) / writeEncrypted(byte[],int,int):
> It's probably slightly faster to replace this kind of loops
> + for (int i = 0; i < cypherText.length / blockSize; i++) {
> + df.decrypt (cypherText, i * blockSize, blockSize, tmpByte,
> + i * blockSize);
> + }
> with
> for (int offset = 0; offset < cypherText.length; offset += blockSize) {
> df.decrypt(cypherText, offset, blockSize, tmpByte, offset);
> }
>Not addressed, I think.
changed
> * LOBFile.readEncrypted(byte[],int,int):
> This part doesn't look correct to me:
> + //find out total number of bytes we can read
> + int newLen = (len - cypherText.length < tailSize)
> + ? len - cypherText.length
> + : tailSize;
> I think it only works if the read starts on a block boundary, and it
> should have used overflow instead of (len - cypherText.length).
>Only partly addressed. I think both occurrences of (len -
>cypherText.length) should have been replaced. Actually, I think it
>could be as simple as newLen = min(overFlow, tailSize).
changed
> * LOBFile.setLength() is a no-op if the new length is greater than the
> old length. Wouldn't it be better to throw an error in this case?
> It's not supposed to happen since it's only called from
> LOBStreamControl.truncate(), but since setLength() is already
> checking that the new size is not greater, I think it's better to
> throw an IllegalArgumentException or UnsupportedOperationException
> than to silently return.
>This seems to have been fixed, but the javadoc still says that it's a
>no-op.
updated javadoc
>A couple of other things I noticed when I looked at the patch again:
> The javadoc comments often just have a @param tag or @throws tag
> which tells the name of the parameter or exception, and doesn't
> describe it. It would be good if they also contained a description.
added description of params and exceptions
> Some of the @throws tags are on this form:
> @throws IOException, SQLException, StandardException
> Here, "SQLException, StandardException" will be interpreted as the
> description of IOException. Please use one @throws tag per
> exception.
done
> The javadoc for LOBFile.seek() says that it's a no-op if pos is
> greater than length. However, it will always modify currentPos, so
> it's not actually a no-op.
fixed also added same code in seek method if new pos is larger than file size
> LOBFile.readEncrypted(byte[],int,int) has this code:
> if (newLen == 0)
> return -1;
> This means -1 is returned if someone requests 0 bytes. I think the
> condition should also include "&& len > 0".
fixed
> When readEncrypted() and writeEncrypted() calculate the value of
> overFlow, I think it would be more readable if it were written as:
> int overFlow = (int) Math.max(0L, currentPos + len - fileLength);
changed
> provide encryption support for temporary files used by lob if the data base
> is encrypted
> ----------------------------------------------------------------------------------------
>
> Key: DERBY-2379
> URL: https://issues.apache.org/jira/browse/DERBY-2379
> Project: Derby
> Issue Type: Sub-task
> Components: JDBC
> Affects Versions: 10.3.0.0
> Environment: all
> Reporter: Anurag Shekhar
> Assigned To: Anurag Shekhar
> Attachments: CosmeticComments_1.txt, derby-2379.diff,
> derby-2379v2-conflict.diff, derby-2379v2.diff, derby-2379v3.diff
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
