On Wed, Jun 12, 2013 at 02:18:43PM -0700, Justin Pettit wrote:
>
> 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.
I changed it to a #define.
> > 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.
I added:
/* 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 OFP_FLOW_REMOVED_REASON_BUFSIZE. */
How's that?
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
