On Jun 5, 2013, at 1:05 PM, Ben Pfaff <b...@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


_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to