LinuxFile reused scratch buffers ensuring size was usable. But the … #331
Conversation
…imit value cannot be modified so later usage failed as an intended overwrite
There was a problem hiding this comment.
You test if the buffer is not null, and then remove it, creating a new buffer. Further you set the byte ordering, but since it's a new byte buffer, i think that is not necessary.
I think we could reduce the code to this:
final int byteSize = size * 4;
if (byteSize > localBufferSize)
throw new ScratchBufferOverrun();
IntBuffer buf = ByteBuffer.allocateDirect(localBufferSize).asIntBuffer();
localOffsetsBuffer.set(buf);
return buf;
Do you agree?
|
Further if i look at the old code, then we reuse the buffer... Are we sure this is ok, to just always write a new buffer? Are we not using the buffers in multiple calls in some way? |
|
My last sentence in the pr about further validation for reuse. We could test if the limit is the same or now less and i think reuse is ok. I would need to decide if there are other attributes that require similar validation. And your code suggestion. I must retest but i think this will not handle the new limit being greater. Will be late my daytime before i can do this work |
…t the limit value cannot be modified so later usage failed as an intended overwrite
…t the buffer limit value cannot be modified so later usage failed as an intended overwrite when the new limitvalue was greater than the old limt value
…t the buffer limit value cannot be modified so later usage failed as an intended overwrite when the new limitvalue was greater than the old limt value
| @@ -214,7 +214,8 @@ protected synchronized IntBuffer getOffsetsBuffer(int size) { | |||
|
|
|||
| if (byteSize > localBufferSize) | |||
| throw new ScratchBufferOverrun(); | |||
|
|
|||
| // if no buffer currently exists, new one always created. | |||
| // if buffer exists and limit is not equal to this getOffsetsBuffer, allocate new | |||
| if (buf == null) { | |||
| ByteBuffer bb = ByteBuffer.allocateDirect(localBufferSize); | |||
|
|
|||
There was a problem hiding this comment.
Robert. U asked about reducing this code. The nativeOrder does alter the buffer and should remain. I did not challenge the comment that nativeOrder be set prior to making the buffer an IntBuff
|
-Git repeated saying that the push failed for my local being behind. Seems it did accomplish something each attempt until I figured out how to fix this without using some dangerous force. That left these extra commits, sorry about that. |
…limit value cannot be modified so later usage failed as an intended overwrite
Modified both methods that return a buffer. The methods ensured the an existing buffer was large enough for the new request and returned it. But its new usage may containa greater limit value. I could not alter the limit value of an existing buffer, instead if an old buffer exists I remove it an allocate a new buffer.
This is an answer to a post. If you agree with this, can you spin a 2.5.1 snap shot so this user can progress.
I did not research how many attributes we needed to compare if we wanted to reuse an existing buffer
@eitch @FDelporte