With a note and a question below:
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Feb 19, 2016, at 12:34 AM, Ben Pfaff <b...@ovn.org> wrote:
>
> A number of times I've looked at code and thought that it would be easier
> to understand if I could write an initializer instead of
> ofpbuf_use_const(). This commit adds a function for that purpose and
> adapts a lot of code to use it, in the places where I thought it made
> the code better.
>
> In theory this could improve code generation since the new function can
> be inlined whereas ofpbuf_use_const() isn't. But I guess that's probably
> insignificant; the intent of this change is code readability.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> —
>
(snip)
> diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
> index dd72cde..489a543 100644
> --- a/lib/ofpbuf.h
> +++ b/lib/ofpbuf.h
> @@ -64,8 +64,9 @@ struct ofpbuf {
> enum ofpbuf_source source; /* Source of memory allocated as 'base'. */
> };
>
> -/* An initializer for a struct ofpbuf that will be initially empty and
> - * uses the space in STUB (which should be an array) as a stub.
> +/* An initializer for a struct ofpbuf that will be initially empty and uses
> the
> + * space in STUB (which should be an array) as a stub. This is the
> initializer
> + * form of ofpbuf_use_stub().
> *
> * Usage example:
> *
> @@ -83,6 +84,27 @@ struct ofpbuf {
> .source = OFPBUF_STUB, \
> }
>
> +/* An initializer for a struct ofpbuf whose data starts at DATA and continues
> + * for SIZE bytes. This is appropriate for an ofpbuf that will be used to
> + * inspect existing data, without moving it around or reallocating it, and
> + * generally without modifying it at all. This is the initializer form of
> + * ofpbuf_use_const().
> + */
> +static inline struct ofpbuf
> +ofpbuf_const_initializer(const void *data, size_t size)
> +{
> + return (struct ofpbuf) {
> + .base = CONST_CAST(void *, data),
> + .data = CONST_CAST(void *, data),
> + .size = size,
> + .allocated = size,
> + .header = NULL,
> + .msg = NULL,
> + .list_node = OVS_LIST_POISON,
> + .source = OFPBUF_STACK,
> + };
> +}
> +
I’m curious if there is a specific reason to define ofpbuf_const_initializer()
as an inline function, and keep the OFPBUF_STUB_INITIALIZER() as a macro?
> void ofpbuf_use_ds(struct ofpbuf *, const struct ds *);
> void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
> void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
(snip)
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 97b7f6c..7b7532d 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -70,6 +70,15 @@ set_switch_config(struct rconn *swconn,
> queue_msg(request);
> }
>
> +unsigned int init_ofpbuf(const void *p, size_t size);
> +unsigned int
> +init_ofpbuf(const void *p, size_t size)
> +{
> + struct ofpbuf b = ofpbuf_const_initializer(p, size);
> + ofpbuf_pull(&b, 8);
> + return b.size;
> +}
> +
This seems unused.
> static void
> process_packet_in(struct controller_ctx *ctx OVS_UNUSED,
> const struct ofp_header *msg)
(snip)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
