[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] issue #2147 - v2

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-01-28 18:27:42 CET

VK Sameer <sameer@collab.net> writes:

> +const char *
> +svn_xml_fuzzy_escape (const char *string, apr_pool_t *pool)
> +{
> + const char *end = string + strlen (string);
> + const char *p = string, *q;
> + svn_stringbuf_t *outstr;
> + char escaped_char[5]; /* 0 x \xdigit \xdigit \0 */
>
> + for (q = p; q < end; q++)
> + if (svn_ctype_iscntrl (*q))

Why is this test not the inverse of the test below?

> + break;
> +
> + /* return original string if no control characters found*/
> + if (q == end)
> + return string;
> +
> + outstr = svn_stringbuf_create ("", pool);
> + while (1)
> + {
> + q = p;
> +
> + /* Traverse till either control character or eos.
> + Skip whitespace control characters */
> + while (q < end &&
> + (svn_ctype_isspace (*q) || !svn_ctype_iscntrl (*q)))

Why is this test not the inverse of the one above?

Perhaps you could structure the code so that the test only occurs in
one place?

> + q++;
> +
> + /* copy chunk before marker */
> + svn_stringbuf_appendbytes (outstr, p, q - p);
> +
> + if (q == end)
> + break;
> +
> + /* Append the control character in 0x%x format. */
> + sprintf (escaped_char, "0x%x", *q);

Plain char could be signed and so the code has the potential to go
wrong if somebody ever modifies the isspace/iscntrl test, casting *q
to (unsigned char) is safer.

> + svn_stringbuf_appendcstr (outstr, escaped_char);
> +
> + p = q + 1;
> + }
> +
> + return outstr->data;
> +}
> +

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 28 18:28:57 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.