[ovs-dev] [bug15637 1/5] json: New function json_serialized_length().

Ben Pfaff blp at nicira.com
Mon Apr 1 20:16:46 UTC 2013


On Mon, Apr 01, 2013 at 09:41:19AM -0700, Justin Pettit wrote:
> On Mar 27, 2013, at 2:44 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > +static size_t
> > +json_string_serialized_length(const char *string)
> > +{
> > +    size_t length;
> > +    uint8_t c;
> > +
> > +    /* "" */
> > +    length = 2;
> 
> I think this length represents the surrounding quotes, but until I
> looked at json_serialize_string(), I thought it was for just a
> single quote based on this comment.

I'm not sure I would make it clearer.  strlen("\"\"")?  I'll go with
that.

> > +/* Returns strlen(json_to_string(json, 0)).
> > + *
> > + * JSSF_SORT does not affect the length of json_to_string()'s output, but
> > + * JSSF_PRETTY does. */
> 
> Is it worth mentioning that this is treating it as not pretty?

The "0" implies that, but maybe that's not obvious.  I changed the
comment to:

/* Returns strlen(json_to_string(json, 0)), that is, the number of bytes in the
 * JSON output by json_to_string() for 'json' when JSSF_PRETTY is not
 * requested.  (JSSF_SORT does not affect the length of json_to_string()'s
 * output.) */

> > +size_t
> > +json_serialized_length(const struct json *json)
> > +{
> > +    switch (json->type) {
> > +    case JSON_NULL:
> > +        return strlen("null");
> > +        break;
> > +
> > +    case JSON_FALSE:
> > +        return strlen("false");
> > +        break;
> > +
> > +    case JSON_TRUE:
> > +        return strlen("true");
> > +        break;
> > +
> > +    case JSON_OBJECT:
> > +        return json_object_serialized_length(json->u.object);
> > +
> > +    case JSON_ARRAY:
> > +        return json_array_serialized_length(&json->u.array);
> > +
> > +    case JSON_INTEGER:
> > +        return snprintf(NULL, 0, "%lld", json->u.integer);
> > +
> > +    case JSON_REAL:
> > +        return snprintf(NULL, 0, "%.*g", DBL_DIG, json->u.real);
> > +
> > +    case JSON_STRING:
> > +        return json_string_serialized_length(json->u.string);
> > +
> > +    case JSON_N_TYPES:
> > +    default:
> > +        NOT_REACHED();
> > +    }
> > +}
> 
> Is there a reason for the inconsistency with some case blocks have a
> "break" and others not?  The json_serialize() function uses "break"
> in all of them.

The json_serialize() cases don't end with "return" statements.

I removed the "break" statements here.

Thank you for the review.



More information about the dev mailing list