[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