aherbert commented on code in PR #135:
URL: https://github.com/apache/commons-codec/pull/135#discussion_r895118797
##########
src/main/java/org/apache/commons/codec/binary/Base16.java:
##########
@@ -177,26 +177,24 @@ void decode(final byte[] data, int offset, final int
length, final Context conte
if (dataLen < availableChars) {
// we have 1/2 byte from previous invocation to decode
result = (context.ibitWorkArea - 1) << BITS_PER_ENCODED_BYTE;
- result |= decodeOctet(data[offset++]);
Review Comment:
Can this avoid use of `offset + i++`:
```Java
result |= decodeOctet(data[offset++]);
i = 1;
```
##########
src/main/java/org/apache/commons/codec/binary/Base16.java:
##########
@@ -177,26 +177,24 @@ void decode(final byte[] data, int offset, final int
length, final Context conte
if (dataLen < availableChars) {
// we have 1/2 byte from previous invocation to decode
result = (context.ibitWorkArea - 1) << BITS_PER_ENCODED_BYTE;
- result |= decodeOctet(data[offset++]);
- i = 2;
+ result |= decodeOctet(data[offset + i++]);
buffer[context.pos++] = (byte)result;
// reset to empty-value for next invocation!
context.ibitWorkArea = 0;
}
- while (i < charsToProcess) {
- result = decodeOctet(data[offset++]) << BITS_PER_ENCODED_BYTE;
- result |= decodeOctet(data[offset++]);
- i += 2;
+ while (i + 1 < charsToProcess) {
Review Comment:
I would prefer to not use `i + 1 < charsToProcess` on the loop condition.
It seems the logic setting `charsToProcess` is incorrect when there is a
previous half-byte. The value should not be final and can be decremented by 1
if `i` is set to 1:
```
result |= decodeOctet(data[offset++]);
i = 1;
charsToProcess--;
// ...
while (i < charsToProcess) {
```
This corrected logic still requires that offset is added to i: `data[offset
+ i++]`. Previously offset was incremented.
This may be better served by computing the end point for offset:
```Java
// End is an even number of chars added to the current offset
final int end = offset + (dataLen & ~0x1);
while (offset < end) {
result = decodeOctet(data[offset++]) << BITS_PER_ENCODED_BYTE;
result |= decodeOctet(data[offset++]);
}
if (offset < dataLen) {
```
This logic would eliminate the requirement for `i` and `charsToProcess`. The
logic to handle the ensureBufferSize can just use `availableChars`. Note: The
code is untested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
