Add unsafe operations on the RawCanChannel and cleanup the buffer ord…

…ering This is primarily to better support downstream libraries. See #22
pschichtel · Aug 31, 2020 · 0edb265 · 0edb265
1 parent 3d5f01a
commit 0edb265
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 35 deletions.
diff --git a/src/main/java/tel/schich/javacan/BcmCanChannel.java b/src/main/java/tel/schich/javacan/BcmCanChannel.java
@@ -27,7 +27,6 @@
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.nio.channels.NotYetBoundException;
-import java.nio.channels.spi.SelectorProvider;
 
 import tel.schich.javacan.linux.LinuxNetworkDevice;
 
@@ -100,7 +99,7 @@ public BcmCanChannel connect(NetworkDevice device) throws IOException {
      * @throws IOException if the socket is not readable
      */
     public BcmMessage read() throws IOException {
-        ByteBuffer frameBuf = ByteBuffer.allocateDirect(MTU);
+        ByteBuffer frameBuf = JavaCAN.allocateOrdered(MTU);
         return read(frameBuf);
     }
 

diff --git a/src/main/java/tel/schich/javacan/BcmMessage.java b/src/main/java/tel/schich/javacan/BcmMessage.java
@@ -23,7 +23,6 @@
 package tel.schich.javacan;
 
 import java.nio.ByteBuffer;
-import java.nio.ByteOrder;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -152,10 +151,9 @@ public BcmMessage(BcmOpcode opcode, Set<BcmFlag> flags, int count, Duration inte
         int frameLength = frameLength(flags);
         base = 0;
         size = HEADER_LENGTH + frames.size() * frameLength;
-        buffer = ByteBuffer.allocateDirect(size);
+        buffer = JavaCAN.allocateOrdered(size);
 
-        buffer.order(ByteOrder.nativeOrder())
-                .putInt(OFFSET_OPCODE, opcode.nativeOpcode)
+        buffer.putInt(OFFSET_OPCODE, opcode.nativeOpcode)
                 .putInt(OFFSET_FLAGS, BcmFlag.toNative(flags))
                 .putInt(OFFSET_COUNT, count)
                 .putInt(OFFSET_CAN_ID, canId)
@@ -310,7 +308,7 @@ public ByteBuffer getBuffer() {
     }
 
     private ByteBuffer createFrameBuffer(int frameIndex, int frameLength) {
-        ByteBuffer frameBuffer = buffer.duplicate();
+        ByteBuffer frameBuffer = buffer.duplicate().order(buffer.order());
         frameBuffer.position(base + OFFSET_FRAMES + frameIndex * frameLength)
                 .limit(frameBuffer.position() + frameLength);
         return frameBuffer;

diff --git a/src/main/java/tel/schich/javacan/CanFrame.java b/src/main/java/tel/schich/javacan/CanFrame.java
@@ -386,15 +386,14 @@ public static CanFrame createRaw(int id, byte flags, byte[] data, int offset, in
         } else {
             bufSize = RawCanChannel.FD_MTU;
         }
-        ByteBuffer buf = ByteBuffer.allocateDirect(bufSize);
-        buf.order(ByteOrder.nativeOrder())
-            .putInt(id)
-            .put((byte) length)
-            .put(flags)
-            .putShort((short) 0) // skip 2 bytes
-            .put(data, offset, length)
-            .clear();
-        return CanFrame.create(buf);
+        ByteBuffer buffer = JavaCAN.allocateOrdered(bufSize);
+        buffer.putInt(id)
+                .put((byte) length)
+                .put(flags)
+                .putShort((short) 0) // skip 2 bytes
+                .put(data, offset, length)
+                .clear();
+        return CanFrame.create(buffer);
     }
 
     /**
@@ -426,6 +425,9 @@ public static CanFrame create(ByteBuffer buffer) {
      * @return the newly created frame
      */
     public static CanFrame createUnsafe(ByteBuffer buffer) {
+        if (buffer.order() != ByteOrder.nativeOrder()) {
+            throw new IllegalArgumentException("byte order (" + buffer.order() + ") of the given buffer must be the native order (" + ByteOrder.nativeOrder() + ")!");
+        }
         int length = buffer.remaining();
         // does the buffer slice size match the non-FD or FD MTU?
         if (length != RawCanChannel.MTU && length != RawCanChannel.FD_MTU) {

diff --git a/src/main/java/tel/schich/javacan/CanSocketOptions.java b/src/main/java/tel/schich/javacan/CanSocketOptions.java
@@ -28,7 +28,6 @@
 import java.nio.ByteOrder;
 import java.time.Duration;
 
-import tel.schich.javacan.linux.LinuxNativeOperationException;
 import tel.schich.javacan.linux.LinuxSocketOptionHandler;
 import tel.schich.javacan.option.CanSocketOption;
 
@@ -127,8 +126,7 @@ public Integer get(int sock) throws IOException {
     public static final SocketOption<CanFilter[]> FILTER = new CanSocketOption<>("FILTER", CanFilter[].class, new LinuxSocketOptionHandler<CanFilter[]>() {
         @Override
         public void set(int sock, CanFilter[] val) throws IOException {
-            ByteBuffer filterData = ByteBuffer.allocateDirect(val.length * CanFilter.BYTES);
-            filterData.order(ByteOrder.nativeOrder());
+            ByteBuffer filterData = JavaCAN.allocateOrdered(val.length * CanFilter.BYTES);
             for (CanFilter f : val) {
                 filterData.putInt(f.getId());
                 filterData.putInt(f.getMask());

diff --git a/src/main/java/tel/schich/javacan/IsotpCanChannel.java b/src/main/java/tel/schich/javacan/IsotpCanChannel.java
@@ -103,6 +103,6 @@ public IsotpCanChannel(int sock) {
      * @return the newly allocated buffer
      */
     public static ByteBuffer allocateSufficientMemory() {
-        return ByteBuffer.allocateDirect(MAX_MESSAGE_LENGTH + 1);
+        return JavaCAN.allocateUnordered(MAX_MESSAGE_LENGTH + 1);
     }
 }
diff --git a/src/main/java/tel/schich/javacan/JavaCAN.java b/src/main/java/tel/schich/javacan/JavaCAN.java
@@ -24,6 +24,8 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
@@ -82,4 +84,24 @@ public synchronized static void initialize() {
 
         initialized = true;
     }
+
+    /**
+     * A simple helper to allocate a {@link ByteBuffer} as needed by the underlying native code.
+     *
+     * @param capacity the capacity of the buffer.
+     * @return the buffer in native byte order with the given capacity.
+     */
+    public static ByteBuffer allocateOrdered(int capacity) {
+        return allocateUnordered(capacity).order(ByteOrder.nativeOrder());
+    }
+
+    /**
+     * A simple helper to allocate a {@link ByteBuffer} as needed by the underlying native code.
+     *
+     * @param capacity the capacity of the buffer.
+     * @return the buffer in default (unspecified) byte order with the given capacity.
+     */
+    public static ByteBuffer allocateUnordered(int capacity) {
+        return ByteBuffer.allocateDirect(capacity);
+    }
 }
diff --git a/src/main/java/tel/schich/javacan/RawCanChannel.java b/src/main/java/tel/schich/javacan/RawCanChannel.java
@@ -47,13 +47,64 @@ public RawCanChannel(int sock) {
 
     public abstract RawCanChannel bind(NetworkDevice device) throws IOException;
 
+    /**
+     * Reads a CAM frame from the channel by internally allocating a new direct {@link ByteBuffer}.
+     *
+     * @return the CAN frame
+     * @throws IOException if the IO operations failed or invalid data was read.
+     */
     public abstract CanFrame read() throws IOException;
+
+    /**
+     * Reads a CAM frame from the channel using the supplied buffer.
+     *
+     * @param buffer the buffer to read into.The buffer's {@link ByteOrder} will be set to native and it will be
+     *               flipped after the read has been completed.
+     * @return the CAN frame
+     * @throws IOException if the IO operations failed, the supplied buffer was insufficient or invalid data was read.
+     */
     public abstract CanFrame read(ByteBuffer buffer) throws IOException;
+
+    /**
+     * Reads raw bytes from the channel.
+     *
+     * This method does not apply any checks on the data that has been read or on the supplied buffer. This method
+     * is primarily intended for downstream libraries that implement their own parsing on the data from the socket.
+     *
+     * @param buffer the buffer to read into.The buffer's {@link ByteOrder} will be set to native and it will be
+     *               flipped after the read has been completed.
+     * @return the number of bytes
+     * @throws IOException if the IO operations failed.
+     */
+    public abstract long readUnsafe(ByteBuffer buffer) throws IOException;
+
+    /**
+     * Writes the given CAN frame.
+     *
+     * @param frame the frame to be written.
+     * @return fluent interface.
+     * @throws IOException if the IO operations failed.
+     */
     public abstract RawCanChannel write(CanFrame frame) throws IOException;
 
+    /**
+     * Writes the given buffer in its entirety to the socket.
+     *
+     * This method does not apply any checks on the given buffer. This method is primarily intended for downstream libraries
+     * that create these buffers using other facilities.
+     *
+     * @param buffer the buffer to be written.
+     * @return the bytes written.
+     * @throws IOException if the IO operations failed.
+     */
+    public abstract long writeUnsafe(ByteBuffer buffer) throws IOException;
+
+    /**
+     * Allocates a buffer that is large enough to hold any supported CAN frame.
+     *
+     * @return a new buffer ready to be used.
+     */
     public static ByteBuffer allocateSufficientMemory() {
-        ByteBuffer buf = ByteBuffer.allocateDirect(FD_MTU + 1);
-        buf.order(ByteOrder.nativeOrder());
-        return buf;
+        return JavaCAN.allocateOrdered(FD_MTU + 1);
     }
 }
diff --git a/src/main/java/tel/schich/javacan/RawCanChannelImpl.java b/src/main/java/tel/schich/javacan/RawCanChannelImpl.java
@@ -24,7 +24,6 @@
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.nio.ByteOrder;
 import java.nio.channels.NotYetBoundException;
 
 import tel.schich.javacan.linux.LinuxNetworkDevice;
@@ -66,25 +65,35 @@ public boolean isBound() {
     @Override
     public CanFrame read() throws IOException {
         int length = getOption(CanSocketOptions.FD_FRAMES) ? FD_MTU : MTU;
-        ByteBuffer frameBuf = ByteBuffer.allocateDirect(length);
+        ByteBuffer frameBuf = JavaCAN.allocateOrdered(length);
         return read(frameBuf);
     }
 
     @Override
     public CanFrame read(ByteBuffer buffer) throws IOException {
-        buffer.order(ByteOrder.nativeOrder());
-        readSocket(buffer);
-        buffer.flip();
+        readUnsafe(buffer);
         return CanFrame.create(buffer);
     }
 
+    @Override
+    public long readUnsafe(ByteBuffer buffer) throws IOException {
+        long bytesRead = readSocket(buffer);
+        buffer.flip();
+        return bytesRead;
+    }
+
     @Override
     public RawCanChannel write(CanFrame frame) throws IOException {
-        long written = writeSocket(frame.getBuffer());
+        long written = writeUnsafe(frame.getBuffer());
         if (written != frame.getSize()) {
             throw new IOException("Frame written incompletely!");
         }
 
         return this;
     }
+
+    @Override
+    public long writeUnsafe(ByteBuffer buffer) throws IOException {
+        return writeSocket(buffer);
+    }
 }
diff --git a/src/test/java/tel/schich/javacan/test/BcmCanSocketTest.java b/src/test/java/tel/schich/javacan/test/BcmCanSocketTest.java
@@ -81,9 +81,7 @@ void testMessageBufferTooSmall() {
         data[BcmMessage.OFFSET_NFRAMES] = frameCount;
         ByteBuffer buffer = ByteBuffer.wrap(data).order(ByteOrder.nativeOrder());
 
-        assertThrows(IllegalArgumentException.class, () -> {
-            new BcmMessage(buffer);
-        });
+        assertThrows(IllegalArgumentException.class, () -> new BcmMessage(buffer));
     }
 
     @Test

diff --git a/src/test/java/tel/schich/javacan/test/CanTestHelper.java b/src/test/java/tel/schich/javacan/test/CanTestHelper.java
@@ -24,6 +24,7 @@
 
 import org.junit.jupiter.api.function.Executable;
 import tel.schich.javacan.CanFrame;
+import tel.schich.javacan.JavaCAN;
 import tel.schich.javacan.NetworkDevice;
 
 import java.io.IOException;
@@ -47,7 +48,7 @@ public static void sendFrameViaUtils(NetworkDevice device, CanFrame frame) throw
         if (frame.isRemoteTransmissionRequest()) {
             data.append('R');
         }
-        ByteBuffer buf = ByteBuffer.allocateDirect(CanFrame.MAX_FD_DATA_LENGTH);
+        ByteBuffer buf = JavaCAN.allocateOrdered(CanFrame.MAX_FD_DATA_LENGTH);
         frame.getData(buf);
         buf.flip();
         while (buf.hasRemaining()) {

diff --git a/src/test/java/tel/schich/javacan/test/RawCanSocketTest.java b/src/test/java/tel/schich/javacan/test/RawCanSocketTest.java
@@ -26,6 +26,7 @@
 import tel.schich.javacan.CanChannels;
 import tel.schich.javacan.CanFilter;
 import tel.schich.javacan.CanFrame;
+import tel.schich.javacan.JavaCAN;
 import tel.schich.javacan.RawCanChannel;
 import tel.schich.javacan.linux.LinuxNativeOperationException;
 
@@ -221,7 +222,7 @@ void testBufferReuseWithNonZeroBase() {
         CanFrame frame = CanFrame.createExtended(0x7FFFFF, FD_NO_FLAGS, data);
         ByteBuffer buffer = frame.getBuffer();
 
-        ByteBuffer largeForReuse = ByteBuffer.allocateDirect(2 * RawCanChannel.FD_MTU);
+        ByteBuffer largeForReuse = JavaCAN.allocateOrdered(2 * RawCanChannel.FD_MTU);
         largeForReuse.position(RawCanChannel.FD_MTU);
         largeForReuse.put(buffer);
         largeForReuse.position(RawCanChannel.FD_MTU);

diff --git a/src/test/java/tel/schich/javacan/test/isotp/IsotpCanSocketOptionsTest.java b/src/test/java/tel/schich/javacan/test/isotp/IsotpCanSocketOptionsTest.java
@@ -79,7 +79,7 @@ void testWriteRead() throws Exception {
                 b.bind(CAN_INTERFACE, dst, src);
 
                 byte[] in = {1, 2, 3, 4};
-                ByteBuffer buf = ByteBuffer.allocateDirect(10);
+                ByteBuffer buf = JavaCAN.allocateUnordered(10);
                 buf.put(in);
                 buf.rewind();
                 final int bytesWritten = a.write(buf);