On Thursday 27 August 2009, George Spelvin wrote:
> Since the buffer size is a power of 2, we can index into it with
> offsets rather than pointers and produce neater code. This
> also lets us use every byte in the buffer, rather than having to
> reserve one to distinguish empty from free.
And *THIS* is what I really wanted to see split up:
(a) hard-wiring BUF_SIZE, and (b) changing the innards.
(a) is going to be obvious.
(b) won't; so it should be very reviewable.
> ---
> drivers/usb/gadget/u_serial.c | 58 +++++++++++++++++------------------------
> 1 files changed, 24 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
> index c44c122..86e9ffc 100644
> --- a/drivers/usb/gadget/u_serial.c
> +++ b/drivers/usb/gadget/u_serial.c
> @@ -76,13 +76,12 @@
> * consider it a NOP. A third layer is provided by the TTY code.
> */
> #define QUEUE_SIZE 16
> -#define WRITE_BUF_SIZE 8192 /* TX only */
> +#define WRITE_BUF_SIZE 8192 /* TX only, must be power of 2 */
>
> /* circular buffer */
> struct gs_buf {
> - char *buf_buf;
> - char *buf_get;
> - char *buf_put;
> + char *buf_buf; /* WRITE_BUF_SIZE */
> + unsigned buf_get, buf_put; /* Offsets into buf_buf */
> };
>
> /*
> @@ -149,8 +148,7 @@ static int gs_buf_alloc(struct gs_buf *gb)
> if (gb->buf_buf == NULL)
> return -ENOMEM;
>
> - gb->buf_put = gb->buf_buf;
> - gb->buf_get = gb->buf_buf;
> + gb->buf_put = gb->buf_get = 0;
>
> return 0;
> }
> @@ -187,7 +185,7 @@ static void gs_buf_clear(struct gs_buf *gb)
> */
> static unsigned gs_buf_data_avail(struct gs_buf const *gb)
> {
> - return (WRITE_BUF_SIZE + gb->buf_put - gb->buf_get) % WRITE_BUF_SIZE;
> + return gb->buf_put - gb->buf_get;
> }
>
> /*
> @@ -198,7 +196,7 @@ static unsigned gs_buf_data_avail(struct gs_buf const *gb)
> */
> static unsigned gs_buf_space_avail(struct gs_buf const *gb)
> {
> - return (WRITE_BUF_SIZE + gb->buf_get - gb->buf_put - 1) % WRITE_BUF_SIZE;
> + return WRITE_BUF_SIZE - gs_buf_data_avail(gb);
> }
>
> /*
> @@ -213,6 +211,7 @@ static unsigned
> gs_buf_put(struct gs_buf *gb, const char *buf, unsigned count)
> {
> unsigned len;
> + unsigned put_pos = gb->buf_put % WRITE_BUF_SIZE;
>
> len = gs_buf_space_avail(gb);
> if (count > len)
> @@ -221,18 +220,17 @@ gs_buf_put(struct gs_buf *gb, const char *buf, unsigned count)
> if (count == 0)
> return 0;
>
> - len = gb->buf_buf + WRITE_BUF_SIZE - gb->buf_put;
> + len = WRITE_BUF_SIZE - put_pos;
> +
> if (count > len) {
> - memcpy(gb->buf_put, buf, len);
> - memcpy(gb->buf_buf, buf+len, count - len);
> - gb->buf_put = gb->buf_buf + count - len;
> + memcpy(gb->buf_buf + put_pos, buf, len);
> + len = count - len;
> + put_pos = 0;
> } else {
> - memcpy(gb->buf_put, buf, count);
> - if (count < len)
> - gb->buf_put += count;
> - else /* count == len */
> - gb->buf_put = gb->buf_buf;
> + len = count;
> }
> + memcpy(gb->buf_buf + put_pos, buf, len);
> + gb->buf_put += count;
>
> return count;
> }
> @@ -249,6 +247,7 @@ static unsigned
> gs_buf_get(struct gs_buf *gb, char *buf, unsigned count)
> {
> unsigned len;
> + unsigned get_pos = gb->buf_get % WRITE_BUF_SIZE;
>
> len = gs_buf_data_avail(gb);
> if (count > len)
> @@ -257,18 +256,16 @@ gs_buf_get(struct gs_buf *gb, char *buf, unsigned count)
> if (count == 0)
> return 0;
>
> - len = gb->buf_buf + WRITE_BUF_SIZE - gb->buf_get;
> + len = WRITE_BUF_SIZE - get_pos;
> if (count > len) {
> - memcpy(buf, gb->buf_get, len);
> - memcpy(buf+len, gb->buf_buf, count - len);
> - gb->buf_get = gb->buf_buf + count - len;
> + memcpy(buf, gb->buf_buf + get_pos, len);
> + len = count - len;
> + get_pos = 0;
> } else {
> - memcpy(buf, gb->buf_get, count);
> - if (count < len)
> - gb->buf_get += count;
> - else /* count == len */
> - gb->buf_get = gb->buf_buf;
> + len = count;
> }
> + memcpy(buf, gb->buf_buf + get_pos, len);
> + gb->buf_get += count;
>
> return count;
> }
> @@ -325,14 +322,7 @@ void gs_free_req(struct usb_ep *ep, struct usb_request *req)
> static unsigned
> gs_send_packet(struct gs_port *port, char *packet, unsigned size)
> {
> - unsigned len;
> -
> - len = gs_buf_data_avail(&port->port_write_buf);
> - if (len < size)
> - size = len;
> - if (size != 0)
> - size = gs_buf_get(&port->port_write_buf, packet, size);
> - return size;
> + return gs_buf_get(&port->port_write_buf, packet, size);
> }
>
> /*
> --
> 1.6.3.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
