Re: [PATCH 2/5] util: Function to parse boolean term queries

Subject: Re: [PATCH 2/5] util: Function to parse boolean term queries

Date: Tue, 25 Dec 2012 10:18:42 -0400

To: Austin Clements, notmuch@notmuchmail.org

Cc:

From: David Bremner


Austin Clements <amdragon@MIT.EDU> writes:

> +    if (consume_double_quote (&pos)) {
> +	char *out = talloc_strdup (ctx, pos);
> +	pos = *term_out = out;
> +	while (1) {

Overall the control flow here is a bit tricky to follow. I'm not sure if
a real loop condition would help or make it worse.

> +	    if (! *pos) {
> +		/* Premature end of string */
> +		goto FAIL;
> +	    } else if (*pos == '"') {
> +		if (*++pos != '"')
> +		    break;
> +	    } else if (consume_double_quote (&pos)) {
> +		break;
> +	    }

I'm confused by the asymmetry here. Quoted strings can start with
unicode quotes, but internally can only have ascii '"'? Is this
documented somewhere?

> +    } else {
> +	while (*pos > ' ' && *pos != ')')
> +	    ++pos;
> +	if (*pos)
> +	    goto FAIL;
> +    }

So if there is no quote, we skip the part after the ':'? I guess I
probably missed something because that doesn't sound like the intended
behaviour.

Thread: