Looks good to me,
Just one question, for the code below in "lib/byteq.c"
"""
/* Returns the number of bytes current queued in 'q'. */
int
byteq_used(const struct byteq *q)
{
return q->head - q->tail;
}
"""
Is it possible that the jsonrpc session last so long that the 'q->head'
gets overflowed? or the chance is so slim that we do not care,
Kind Regards,
Alex Wang,
On Fri, Jul 12, 2013 at 2:54 PM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/byteq.c | 26 +++++++++++++++-----------
> lib/byteq.h | 10 ++++------
> lib/jsonrpc.c | 5 +++--
> 3 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/lib/byteq.c b/lib/byteq.c
> index 2ee4a65..3f865cf 100644
> --- a/lib/byteq.c
> +++ b/lib/byteq.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009, 2012 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2012, 2013 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -20,13 +20,17 @@
> #include <unistd.h>
> #include "util.h"
>
> -/* The queue size must be a power of 2. */
> -BUILD_ASSERT_DECL(!(BYTEQ_SIZE & (BYTEQ_SIZE - 1)));
> -
> -/* Initializes 'q' as empty. */
> +/* Initializes 'q' as an empty byteq that uses the 'size' bytes of
> 'buffer' to
> + * store data. 'size' must be a power of 2.
> + *
> + * The caller must ensure that 'buffer' remains available to the byteq as
> long
> + * as 'q' is in use. */
> void
> -byteq_init(struct byteq *q)
> +byteq_init(struct byteq *q, uint8_t *buffer, size_t size)
> {
> + ovs_assert(is_pow2(size));
> + q->buffer = buffer;
> + q->size = size;
> q->head = q->tail = 0;
> }
>
> @@ -41,7 +45,7 @@ byteq_used(const struct byteq *q)
> int
> byteq_avail(const struct byteq *q)
> {
> - return BYTEQ_SIZE - byteq_used(q);
> + return q->size - byteq_used(q);
> }
>
> /* Returns true if no bytes are queued in 'q',
> @@ -147,7 +151,7 @@ int
> byteq_tailroom(const struct byteq *q)
> {
> int used = byteq_used(q);
> - int tail_to_end = BYTEQ_SIZE - (q->tail & (BYTEQ_SIZE - 1));
> + int tail_to_end = q->size - (q->tail & (q->size - 1));
> return MIN(used, tail_to_end);
> }
>
> @@ -156,7 +160,7 @@ byteq_tailroom(const struct byteq *q)
> const uint8_t *
> byteq_tail(const struct byteq *q)
> {
> - return &q->buffer[q->tail & (BYTEQ_SIZE - 1)];
> + return &q->buffer[q->tail & (q->size - 1)];
> }
>
> /* Removes 'n' bytes from the tail of 'q', which must have at least 'n'
> bytes
> @@ -173,7 +177,7 @@ byteq_advance_tail(struct byteq *q, unsigned int n)
> uint8_t *
> byteq_head(struct byteq *q)
> {
> - return &q->buffer[q->head & (BYTEQ_SIZE - 1)];
> + return &q->buffer[q->head & (q->size - 1)];
> }
>
> /* Returns the number of contiguous bytes of free space starting at the
> head
> @@ -182,7 +186,7 @@ int
> byteq_headroom(const struct byteq *q)
> {
> int avail = byteq_avail(q);
> - int head_to_end = BYTEQ_SIZE - (q->head & (BYTEQ_SIZE - 1));
> + int head_to_end = q->size - (q->head & (q->size - 1));
> return MIN(avail, head_to_end);
> }
>
> diff --git a/lib/byteq.h b/lib/byteq.h
> index 5fa51fd..d73e368 100644
> --- a/lib/byteq.h
> +++ b/lib/byteq.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2013 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -20,17 +20,15 @@
> #include <stddef.h>
> #include <stdint.h>
>
> -/* Maximum number of bytes in a byteq. */
> -#define BYTEQ_SIZE 512
> -
> /* General-purpose circular queue of bytes. */
> struct byteq {
> - uint8_t buffer[BYTEQ_SIZE]; /* Circular queue. */
> + uint8_t *buffer; /* Circular queue. */
> + unsigned int size; /* Number of bytes allocated for
> 'buffer'. */
> unsigned int head; /* Head of queue. */
> unsigned int tail; /* Chases the head. */
> };
>
> -void byteq_init(struct byteq *);
> +void byteq_init(struct byteq *, uint8_t *buffer, size_t size);
> int byteq_used(const struct byteq *);
> int byteq_avail(const struct byteq *);
> bool byteq_is_empty(const struct byteq *);
> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> index 56b4cce..b4bbc84 100644
> --- a/lib/jsonrpc.c
> +++ b/lib/jsonrpc.c
> @@ -41,6 +41,7 @@ struct jsonrpc {
>
> /* Input. */
> struct byteq input;
> + uint8_t input_buffer[512];
> struct json_parser *parser;
> struct jsonrpc_msg *received;
>
> @@ -87,7 +88,7 @@ jsonrpc_open(struct stream *stream)
> rpc = xzalloc(sizeof *rpc);
> rpc->name = xstrdup(stream_get_name(stream));
> rpc->stream = stream;
> - byteq_init(&rpc->input);
> + byteq_init(&rpc->input, rpc->input_buffer, sizeof rpc->input_buffer);
> list_init(&rpc->output);
>
> return rpc;
> @@ -330,7 +331,7 @@ jsonrpc_recv(struct jsonrpc *rpc, struct jsonrpc_msg
> **msgp)
> jsonrpc_received(rpc);
> if (rpc->status) {
> const struct byteq *q = &rpc->input;
> - if (q->head <= BYTEQ_SIZE) {
> + if (q->head <= q->size) {
> stream_report_content(q->buffer, q->head,
> STREAM_JSONRPC,
> THIS_MODULE, rpc->name);
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
