[ovs-dev] [threads 14/17] ofp-util: Don't return static data in ofputil_packet_in_reason_to_string().

[Pravin Shelar]

On Wed, Jun 5, 2013 at 1:05 PM, Ben Pfaff <blp at 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 <blp at 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.
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev