Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 08b678c0 authored by Philip P. Moltmann's avatar Philip P. Moltmann
Browse files

Correctly use ByteBuffer in UsbRequest

Meaning: Read/Write to correct area in buffer, set position correectly.

- Create a new method UsbRequest#enqueue that has correct behavior, deprecate
  UsbRequest#queue.
- Move all description of the weird (legacy) UsbRequest#queue behavior to
  this method.

Change-Id: Ibeed400b4ad2aa9d005ace345c7895a3dc4ba1ad
Fixes: 31050148
Test: Submitted alongside
parent 411f8215
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -14835,10 +14835,11 @@ package android.hardware.usb {
    ctor public UsbRequest();
    method public boolean cancel();
    method public void close();
    method public boolean enqueue(java.nio.ByteBuffer);
    method public java.lang.Object getClientData();
    method public android.hardware.usb.UsbEndpoint getEndpoint();
    method public boolean initialize(android.hardware.usb.UsbDeviceConnection, android.hardware.usb.UsbEndpoint);
    method public boolean queue(java.nio.ByteBuffer, int);
    method public deprecated boolean queue(java.nio.ByteBuffer, int);
    method public void setClientData(java.lang.Object);
  }
+2 −1
Original line number Diff line number Diff line
@@ -16045,10 +16045,11 @@ package android.hardware.usb {
    ctor public UsbRequest();
    method public boolean cancel();
    method public void close();
    method public boolean enqueue(java.nio.ByteBuffer);
    method public java.lang.Object getClientData();
    method public android.hardware.usb.UsbEndpoint getEndpoint();
    method public boolean initialize(android.hardware.usb.UsbDeviceConnection, android.hardware.usb.UsbEndpoint);
    method public boolean queue(java.nio.ByteBuffer, int);
    method public deprecated boolean queue(java.nio.ByteBuffer, int);
    method public void setClientData(java.lang.Object);
  }
+2 −1
Original line number Diff line number Diff line
@@ -14852,10 +14852,11 @@ package android.hardware.usb {
    ctor public UsbRequest();
    method public boolean cancel();
    method public void close();
    method public boolean enqueue(java.nio.ByteBuffer);
    method public java.lang.Object getClientData();
    method public android.hardware.usb.UsbEndpoint getEndpoint();
    method public boolean initialize(android.hardware.usb.UsbDeviceConnection, android.hardware.usb.UsbEndpoint);
    method public boolean queue(java.nio.ByteBuffer, int);
    method public deprecated boolean queue(java.nio.ByteBuffer, int);
    method public void setClientData(java.lang.Object);
  }
+1 −5
Original line number Diff line number Diff line
@@ -265,8 +265,6 @@ public class UsbDeviceConnection {
     * {@link android.hardware.usb.UsbRequest#getEndpoint} and {@link
     * android.hardware.usb.UsbRequest#getClientData} can be useful in determining how to process
     * the result of this function.</p>
     * <p>Position and array offset of the request's buffer are ignored and assumed to be 0. The
     * position will be set to the number of bytes read/written.</p>
     *
     * @return a completed USB request, or null if an error occurred
     *
@@ -291,8 +289,6 @@ public class UsbDeviceConnection {
     * {@link android.hardware.usb.UsbRequest#getEndpoint} and {@link
     * android.hardware.usb.UsbRequest#getClientData} can be useful in determining how to process
     * the result of this function.</p>
     * <p>Position and array offset of the request's buffer are ignored and assumed to be 0. The
     * position will be set to the number of bytes read/written.</p>
     * <p>Android processes {@link UsbRequest UsbRequests} asynchronously. Hence it is not
     * guaranteed that {@link #requestWait(int) requestWait(0)} returns a request that has been
     * queued right before even if the request could have been processed immediately.</p>
@@ -307,7 +303,7 @@ public class UsbDeviceConnection {
     *                                  {@link UsbRequest#queue(ByteBuffer, int)}
     */
    public UsbRequest requestWait(int timeout) {
        timeout = Preconditions.checkArgumentNonnegative(timeout);
        timeout = Preconditions.checkArgumentNonnegative(timeout, "timeout");

        UsbRequest request = native_request_wait(timeout);
        if (request != null) {
+163 −32
Original line number Diff line number Diff line
@@ -16,8 +16,10 @@

package android.hardware.usb;

import android.util.Log;
import android.annotation.Nullable;

import com.android.internal.util.Preconditions;

import dalvik.system.CloseGuard;

import java.nio.ByteBuffer;
@@ -38,13 +40,18 @@ public class UsbRequest {

    private static final String TAG = "UsbRequest";

    // From drivers/usb/core/devio.c
    private static final int MAX_USBFS_BUFFER_SIZE = 16384;

    // used by the JNI code
    private long mNativeContext;

    private UsbEndpoint mEndpoint;

    // for temporarily saving current buffer across queue and dequeue
    /** The buffer that is currently being read / written */
    private ByteBuffer mBuffer;

    /** The amount of data to read / write when using {@link #queue} */
    private int mLength;

    // for client use
@@ -53,8 +60,21 @@ public class UsbRequest {
    // Prevent the connection from being finalized
    private UsbDeviceConnection mConnection;

    /** Whether this buffer was {@link #enqueue enqueued (new behavior)} or {@link #queue queued
     * (deprecared behavior)}. */
    private boolean mIsUsingEnqueue;

    /** Temporary buffer than might be used while buffer is enqueued */
    private ByteBuffer mTempBuffer;

    private final CloseGuard mCloseGuard = CloseGuard.get();

    /**
     * Lock for queue, enqueue and dequeue, so a queue operation can be finished by a dequeue
     * operation on a different thread.
     */
    private final Object mLock = new Object();

    public UsbRequest() {
    }

@@ -67,7 +87,7 @@ public class UsbRequest {
     */
    public boolean initialize(UsbDeviceConnection connection, UsbEndpoint endpoint) {
        mEndpoint = endpoint;
        mConnection = Preconditions.checkNotNull(connection);
        mConnection = Preconditions.checkNotNull(connection, "connection");

        boolean wasInitialized = native_init(connection, endpoint.getAddress(),
                endpoint.getAttributes(), endpoint.getMaxPacketSize(), endpoint.getInterval());
@@ -140,19 +160,26 @@ public class UsbRequest {
     * Queues the request to send or receive data on its endpoint.
     * <p>For OUT endpoints, the given buffer data will be sent on the endpoint. For IN endpoints,
     * the endpoint will attempt to read the given number of bytes into the specified buffer. If the
     * queueing operation is successful, we return true and the result will be returned via {@link
     * android.hardware.usb.UsbDeviceConnection#requestWait}</p>
     * queueing operation is successful, return true. The result will be returned via
     * {@link UsbDeviceConnection#requestWait}</p>
     *
     * @param buffer the buffer containing the bytes to write, or location to store the results of a
     *               read. Position and array offset will be ignored and assumed to be 0. Limit and
     *               capacity will be ignored.
     *               capacity will be ignored. Once the request
     *               {@link UsbDeviceConnection#requestWait() is processed} the position will be set
     *               to the number of bytes read/written.
     * @param length number of bytes to read or write.
     *
     * @return true if the queueing operation succeeded
     *
     * @deprecated Use {@link #enqueue(ByteBuffer)} instead.
     */
    @Deprecated
    public boolean queue(ByteBuffer buffer, int length) {
        boolean out = (mEndpoint.getDirection() == UsbConstants.USB_DIR_OUT);
        boolean result;

        synchronized (mLock) {
            // save our buffer for when the request has completed
            mBuffer = buffer;
            mLength = length;
@@ -160,7 +187,6 @@ public class UsbRequest {
            // Note: On a buffer slice we lost the capacity information about the underlying buffer,
            // hence we cannot check if the access would be a data leak/memory corruption.

        boolean result;
            if (buffer.isDirect()) {
                result = native_queue_direct(buffer, length, out);
            } else if (buffer.hasArray()) {
@@ -172,23 +198,127 @@ public class UsbRequest {
                mBuffer = null;
                mLength = 0;
            }
        }

        return result;
    }

    /**
     * Queues the request to send or receive data on its endpoint.
     *
     * <p>For OUT endpoints, the remaining bytes of the buffer will be sent on the endpoint. For IN
     * endpoints, the endpoint will attempt to fill the remaining bytes of the buffer. If the
     * queueing operation is successful, return true. The result will be returned via
     * {@link UsbDeviceConnection#requestWait}</p>
     *
     * @param buffer the buffer containing the bytes to send, or the buffer to fill. The state
     *               of the buffer is undefined until the request is returned by
     *               {@link UsbDeviceConnection#requestWait}. If the request failed the buffer
     *               will be unchanged; if the request succeeded the position of the buffer is
     *               incremented by the number of bytes sent/received.
     *
     * @return true if the queueing operation succeeded
     */
    public boolean enqueue(@Nullable ByteBuffer buffer) {
        // Request need to be initialized
        Preconditions.checkState(mNativeContext != 0, "request is not initialized");

        // Request can not be currently enqueued
        Preconditions.checkState(!mIsUsingEnqueue, "request is currently enqueued");

        boolean isSend = (mEndpoint.getDirection() == UsbConstants.USB_DIR_OUT);
        boolean wasEnqueued;

        synchronized (mLock) {
            mBuffer = buffer;

            if (buffer == null) {
                // Null buffers enqueue empty USB requests which is supported
                mIsUsingEnqueue = true;
                wasEnqueued = native_enqueue(null, 0, 0);
            } else {
                // Can only send/receive MAX_USBFS_BUFFER_SIZE bytes at once
                Preconditions.checkArgumentInRange(buffer.remaining(), 0, MAX_USBFS_BUFFER_SIZE,
                        "number of remaining bytes");

                // Can not receive into read-only buffers.
                Preconditions.checkArgument(!(buffer.isReadOnly() && !isSend), "buffer can not be "
                        + "read-only when receiving data");

                if (!buffer.isDirect()) {
                    mTempBuffer = ByteBuffer.allocateDirect(mBuffer.remaining());

                    if (isSend) {
                        // Copy buffer into temporary buffer
                        mBuffer.mark();
                        mTempBuffer.put(mBuffer);
                        mTempBuffer.flip();
                        mBuffer.reset();
                    }

                    // Send/Receive into the temp buffer instead
                    buffer = mTempBuffer;
                }

                mIsUsingEnqueue = true;
                wasEnqueued = native_enqueue(buffer, buffer.position(), buffer.remaining());
            }
        }

        if (!wasEnqueued) {
            mIsUsingEnqueue = false;
            mTempBuffer = null;
            mBuffer = null;
        }

        return wasEnqueued;
    }

    /* package */ void dequeue() {
        boolean out = (mEndpoint.getDirection() == UsbConstants.USB_DIR_OUT);
        int bytesRead;
        boolean isSend = (mEndpoint.getDirection() == UsbConstants.USB_DIR_OUT);
        int bytesTransferred;

        synchronized (mLock) {
            if (mIsUsingEnqueue) {
                bytesTransferred = native_dequeue_direct();
                mIsUsingEnqueue = false;

                if (mBuffer == null) {
                    // Nothing to do
                } else if (mTempBuffer == null) {
                    mBuffer.position(mBuffer.position() + bytesTransferred);
                } else {
                    mTempBuffer.limit(bytesTransferred);

                    // The user might have modified mBuffer which might make put/position fail.
                    // Changing the buffer while a request is in flight is not supported. Still,
                    // make sure to free mTempBuffer correctly.
                    try {
                        if (isSend) {
                            mBuffer.position(mBuffer.position() + bytesTransferred);
                        } else {
                            // Copy temp buffer back into original buffer
                            mBuffer.put(mTempBuffer);
                        }
                    } finally {
                        mTempBuffer = null;
                    }
                }
            } else {
                if (mBuffer.isDirect()) {
            bytesRead = native_dequeue_direct();
                    bytesTransferred = native_dequeue_direct();
                } else {
            bytesRead = native_dequeue_array(mBuffer.array(), mLength, out);
                    bytesTransferred = native_dequeue_array(mBuffer.array(), mLength, isSend);
                }
        if (bytesRead >= 0) {
            mBuffer.position(Math.min(bytesRead, mLength));
                if (bytesTransferred >= 0) {
                    mBuffer.position(Math.min(bytesTransferred, mLength));
                }
            }

            mBuffer = null;
            mLength = 0;
        }
    }

    /**
     * Cancels a pending queue operation.
@@ -202,6 +332,7 @@ public class UsbRequest {
    private native boolean native_init(UsbDeviceConnection connection, int ep_address,
            int ep_attributes, int ep_max_packet_size, int ep_interval);
    private native void native_close();
    private native boolean native_enqueue(ByteBuffer buffer, int offset, int length);
    private native boolean native_queue_array(byte[] buffer, int length, boolean out);
    private native int native_dequeue_array(byte[] buffer, int length, boolean out);
    private native boolean native_queue_direct(ByteBuffer buffer, int length, boolean out);
Loading