[pulseaudio-discuss] [PATCH v2 1/9] pulse: Add a JSON-parsing library

[Tanu Kaskinen]

On Sat, 2016-06-04 at 12:43 +0530, Arun Raghavan wrote:
> 
> On Thu, 2 Jun 2016, at 04:41 PM, Tanu Kaskinen wrote:
> > On Wed, 2016-06-01 at 17:18 +0530, Arun Raghavan wrote:
> > > +static const char* parse_string(const char *str, pa_json_object *obj) {
> > > +    pa_strbuf *buf = pa_strbuf_new();
> > > +
> > > +    str++; /* Consume leading '"' */
> > > +
> > > +    while (*str != '"') {
> > > +        if (*str != '\\') {
> > > +            /* We don't accept non-ASCII, non-control characters */
> > 
> > This comment seems to be saying either that we don't accept any
> > printable characters, or that we don't accept non-ASCII printable
> > characters. Rewording suggestion: "We only accept ASCII printable
> > characters."
> > 
> > > +            if (*str < 0x20) {
> > 
> > Should be "if (*str < 0x20 || *str > 0x7E) {"
> 
> Actually, no. That should throw a compiler warning since str is a
> (signed) char and will never be greater than 0x7E. Maybe we can just
> cast to unsigned and do the check so this isn't confusing.

Ah, right, I forgot that chars can be (and often are) signed. It's
actually implementation-defined whether they are signed or not. I think
the code I committed is good anyway, since it will handle both signed
and unsigned cases, and in any case we need a separate check to reject
0x7F, which is a positive number even with signed chars. 0x7F is the
DEL character. 0x7E is the last printable character.

-- 
Tanu