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