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

Ben Pfaff blp at nicira.com
Wed Jun 12 21:44:36 UTC 2013


On Wed, Jun 12, 2013 at 02:18:43PM -0700, Justin Pettit wrote:
> 
> 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.

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.



More information about the dev mailing list