On Fri, Jun 07, 2013 at 02:30:15PM -0700, Pravin Shelar wrote:
> On Wed, Jun 5, 2013 at 1:05 PM, Ben Pfaff <b...@nicira.com> wrote:
> > Returning a static data buffer makes code more brittle and definitely
> > not thread-safe, so this commit switches to using a caller-provided
> > buffer instead.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/ofp-actions.c | 5 ++++-
> > lib/ofp-print.c | 12 +++++++++---
> > lib/ofp-util.c | 19 +++++++++++++------
> > lib/ofp-util.h | 6 +++++-
> > 4 files changed, 31 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> > index c98e29a..d9b503d 100644
> > --- a/lib/ofp-actions.c
> > +++ b/lib/ofp-actions.c
> > @@ -2085,8 +2085,11 @@ ofpact_format(const struct ofpact *a, struct ds *s)
> >
> > ds_put_cstr(s, "controller(");
> > if (reason != OFPR_ACTION) {
> > + char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
> > +
> > ds_put_format(s, "reason=%s,",
> > - ofputil_packet_in_reason_to_string(reason));
> > + ofputil_packet_in_reason_to_string(
> > + reason, reasonbuf, sizeof reasonbuf));
> > }
> > if (controller->max_len != UINT16_MAX) {
> > ds_put_format(s, "max_len=%"PRIu16",",
> > controller->max_len);
> > diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> > index 3f00305..b2370b9 100644
> > --- a/lib/ofp-print.c
> > +++ b/lib/ofp-print.c
> > @@ -86,6 +86,7 @@ static void
> > ofp_print_packet_in(struct ds *string, const struct ofp_header *oh,
> > int verbosity)
> > {
> > + char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
> > struct ofputil_packet_in pin;
> > int error;
> > int i;
> > @@ -130,7 +131,8 @@ ofp_print_packet_in(struct ds *string, const struct
> > ofp_header *oh,
> > }
> >
> > ds_put_format(string, " (via %s)",
> > - ofputil_packet_in_reason_to_string(pin.reason));
> > + ofputil_packet_in_reason_to_string(pin.reason, reasonbuf,
> > + sizeof reasonbuf));
> >
> > ds_put_format(string, " data_len=%zu", pin.packet_len);
> > if (pin.buffer_id == UINT32_MAX) {
> > @@ -1663,8 +1665,12 @@ ofp_print_nxt_set_async_config(struct ds *string,
> > ds_put_cstr(string, " PACKET_IN:");
> > for (j = 0; j < 32; j++) {
> > if (nac->packet_in_mask[i] & htonl(1u << j)) {
> > - ds_put_format(string, " %s",
> > - ofputil_packet_in_reason_to_string(j));
> > + char reasonbuf[OFP_FLOW_REMOVED_REASON_BUFSIZE];
> > + const char *reason;
> > +
> > + reason = ofputil_packet_in_reason_to_string(j, reasonbuf,
> > + sizeof
> > reasonbuf);
> > + ds_put_format(string, " %s", reason);
> > }
> > }
> > if (!nac->packet_in_mask[i]) {
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index 3c8d5a2..24fa5ea 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -2654,11 +2654,13 @@ ofputil_encode_packet_in(const struct
> > ofputil_packet_in *pin,
> > return packet;
> > }
> >
> > +/* Returns a string form of 'reason'. The return value is either a
> > statically
> > + * allocated constant string or the 'bufsize'-byte buffer 'reasonbuf'.
> > + * 'bufsize' should be at least OFPUTIL_PACKET_IN_REASON_BUFSIZE. */
> > const char *
> > -ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason reason)
> > +ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason reason,
> > + char *reasonbuf, size_t bufsize)
> > {
> > - static char s[INT_STRLEN(int) + 1];
> > -
> > switch (reason) {
> > case OFPR_NO_MATCH:
> > return "no_match";
> > @@ -2669,8 +2671,8 @@ ofputil_packet_in_reason_to_string(enum
> > ofp_packet_in_reason reason)
> >
> > case OFPR_N_REASONS:
> > default:
> > - sprintf(s, "%d", (int) reason);
> > - return s;
> > + snprintf(reasonbuf, bufsize, "%d", (int) reason);
> > + return reasonbuf;
> > }
> > }
> >
> > @@ -2681,7 +2683,12 @@ ofputil_packet_in_reason_from_string(const char *s,
> > int i;
> >
> > for (i = 0; i < OFPR_N_REASONS; i++) {
> > - if (!strcasecmp(s, ofputil_packet_in_reason_to_string(i))) {
> > + char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
> > + const char *reason_s;
> > +
> > + reason_s = ofputil_packet_in_reason_to_string(i, reasonbuf,
> > + sizeof reasonbuf);
> > + if (!strcasecmp(s, reason_s)) {
> > *reason = i;
> > return true;
> > }
> > diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> > index 010c34d..1c8d6cd 100644
> > --- a/lib/ofp-util.h
> > +++ b/lib/ofp-util.h
> > @@ -27,6 +27,7 @@
> > #include "netdev.h"
> > #include "openflow/nicira-ext.h"
> > #include "openvswitch/types.h"
> > +#include "type-props.h"
> >
> > struct ofpbuf;
> >
> > @@ -334,7 +335,10 @@ struct ofpbuf *ofputil_encode_packet_in(const struct
> > ofputil_packet_in *,
> > enum ofputil_protocol protocol,
> > enum nx_packet_in_format);
> >
> > -const char *ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason);
> > +enum { OFPUTIL_PACKET_IN_REASON_BUFSIZE = INT_STRLEN(int) + 1 };
> > +const char *ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason,
> > + char *reasonbuf,
> > + size_t bufsize);
> > bool ofputil_packet_in_reason_from_string(const char *,
> > enum ofp_packet_in_reason *);
> >
>
> Looks good.
>
> Thanks.
Thanks, applied to master.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
