[ovs-dev] [threads 13/17] ofp-print: Avoid returning static data.

Justin Pettit jpettit at nicira.com
Wed Jun 12 21:18:43 UTC 2013


On Jun 5, 2013, at 1:05 PM, Ben Pfaff <blp at nicira.com> wrote:

> +enum { OFP_FLOW_REMOVED_REASON_BUFSIZE = INT_STRLEN(int) + 1 };

The use of an enum to define these surprised me.  I don't think it's wrong.  Just a convention I don't remember seeing elsewhere.

> static const char *
> -ofp_flow_removed_reason_to_string(enum ofp_flow_removed_reason reason)
> +ofp_flow_removed_reason_to_string(enum ofp_flow_removed_reason reason,
> +                                  char *reasonbuf, size_t bufsize)
> {
> -    static char s[32];
> -
>     switch (reason) {
>     case OFPRR_IDLE_TIMEOUT:
>         return "idle";
> @@ -874,14 +874,15 @@ ofp_flow_removed_reason_to_string(enum ofp_flow_removed_reason reason)
>     case OFPRR_EVICTION:
>         return "eviction";
>     default:
> -        sprintf(s, "%d", (int) reason);
> -        return s;
> +        snprintf(reasonbuf, bufsize, "%d", (int) reason);
> +        return reasonbuf;
>     }
> }

I think the use of this function is sufficiently complicated that it could use a comment.

> +enum { OFP_PORT_REASON_BUFSIZE = INT_STRLEN(int) + 1 };
> static const char *
> -ofp_port_reason_to_string(enum ofp_port_reason reason)
> +ofp_port_reason_to_string(enum ofp_port_reason reason,
> +                          char *reasonbuf, size_t bufsize)
> {
> -    static char s[32];
> -
>     switch (reason) {
>     case OFPPR_ADD:
>         return "add";
> @@ -1642,8 +1644,8 @@ ofp_port_reason_to_string(enum ofp_port_reason reason)
>         return "modify";
> 
>     default:
> -        sprintf(s, "%d", (int) reason);
> -        return s;
> +        snprintf(reasonbuf, bufsize, "%d", (int) reason);
> +        return reasonbuf;
>     }
> }

Same here.

--Justin





More information about the dev mailing list