Copilot commented on code in PR #784:
URL: https://github.com/apache/commons-io/pull/784#discussion_r2367858008
##########
src/main/java/org/apache/commons/io/channels/ByteArraySeekableByteChannel.java:
##########
@@ -37,41 +37,66 @@
* accessed via {@link ByteArraySeekableByteChannel#array()}.
* </p>
*
- * @since 2.19.0
+ * @since 2.21.0
*/
public class ByteArraySeekableByteChannel implements SeekableByteChannel {
private static final int RESIZE_LIMIT = Integer.MAX_VALUE >> 1;
+
+ /**
+ * Constructs a channel that wraps the given byte array.
+ * <p>
+ * The resulting channel will share the given array as its buffer, until a
write operation
+ * requires a larger capacity.
+ * The initial size of the channel is the length of the given array, and
the initial position is 0.
+ * </p>
+ * @param bytes The byte array to wrap; must not be {@code null}.
+ * @return A new channel that wraps the given byte array; never {@code
null}.
+ * @throws NullPointerException If the byte array is {@code null}.
+ */
+ public static ByteArraySeekableByteChannel wrap(byte[] bytes) {
+ Objects.requireNonNull(bytes, "bytes");
+ return new ByteArraySeekableByteChannel(bytes, bytes.length);
+ }
+
private byte[] data;
- private final AtomicBoolean closed = new AtomicBoolean();
+ private volatile boolean closed;
private int position;
private int size;
private final ReentrantLock lock = new ReentrantLock();
/**
* Constructs a new instance using a default empty buffer.
+ * <p>
+ * The initial size of the channel is 0, and the initial position is 0.
+ * </p>
*/
public ByteArraySeekableByteChannel() {
this(0);
}
- /**
- * Constructs a new instance from a byte array.
- *
- * @param data input data or pre-allocated array.
- */
- public ByteArraySeekableByteChannel(final byte[] data) {
- this.data = data;
- this.size = data.length;
- }
-
/**
* Constructs a new instance from a size of storage to be allocated.
- *
+ * <p>
+ * The initial size of the channel is 0, and the initial position is 0.
+ * </p>
* @param size size of internal buffer to allocate, in bytes.
*/
public ByteArraySeekableByteChannel(final int size) {
- this(new byte[size]);
+ this(byteArray(size), 0);
+ }
+
+ private static byte[] byteArray(int value) {
+ if (value < 0) {
+ throw new IllegalArgumentException("Size must be non-negative");
+ }
+ return new byte[value];
+ }
+
+ private ByteArraySeekableByteChannel(byte[] data, int size) {
Review Comment:
The private constructor doesn't validate that the size parameter is within
bounds of the data array length. Consider adding validation to ensure size <=
data.length to prevent inconsistent internal state.
```suggestion
private ByteArraySeekableByteChannel(byte[] data, int size) {
if (size < 0 || size > data.length) {
throw new IllegalArgumentException("Size must be non-negative
and less than or equal to data.length (size=" + size + ", data.length=" +
data.length + ")");
}
```
##########
src/main/java/org/apache/commons/io/channels/ByteArraySeekableByteChannel.java:
##########
@@ -212,13 +249,12 @@ public int write(final ByteBuffer b) throws IOException {
checkOpen();
lock.lock();
try {
- int wanted = b.remaining();
- final int possibleWithoutResize = size - position;
+ final int wanted = b.remaining();
+ final int possibleWithoutResize = Math.max(0, size - position);
Review Comment:
The calculation `Math.max(0, size - position)` is unnecessary since position
should never exceed size in a properly functioning channel. Consider using a
simpler calculation or adding an assertion to verify this invariant.
```suggestion
assert position <= size : "Channel position exceeds size";
final int possibleWithoutResize = size - position;
```
##########
src/main/java/org/apache/commons/io/build/AbstractOrigin.java:
##########
@@ -324,6 +589,18 @@ public Reader getReader(final Charset charset) throws
IOException {
return new InputStreamReader(getInputStream(),
Charsets.toCharset(charset));
}
+ @Override
+ public ReadableByteChannel getReadableByteChannel(OpenOption...
options) throws IOException {
+ return Channels.newChannel(getInputStream(options));
+ }
+
+ public long size() throws IOException {
+ if (origin instanceof FileInputStream) {
+ final FileInputStream fileInputStream = (FileInputStream)
origin;
+ return fileInputStream.getChannel().size();
+ }
+ throw unsupportedOperation("size");
+ }
Review Comment:
The size() method in InputStreamOrigin is missing the @Override annotation.
This should be added for clarity and to ensure proper method overriding.
--
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]
