On Wed, Aug 31, 2011 at 10:25:34AM +0000, Denys Vlasenko wrote: > commit 8778bffdd25dca050b3aa2a7a7e05bc8a63a6665 > Author: Denys Vlasenko <[email protected]> > Date: Wed Aug 31 12:22:56 2011 +0200 > > Optimize string_quote() for speed > > * util.c (string_quote): Speed up check for terminating NUL. > Replace strintf() with open-coded binary to hex/oct conversions - > we potentially do them for every single byte, need to be fast.
The change is mostly OK, but
> --- a/util.c
> +++ b/util.c
> @@ -440,8 +440,15 @@ string_quote(const char *instr, char *outstr, int len,
> int size)
> {
> const unsigned char *ustr = (const unsigned char *) instr;
> char *s = outstr;
> - int usehex = 0, c, i;
> + int usehex, c, i, eol;
>
> + eol = 0x100; /* this can never match a char */
> + if (len < 0) {
> + size--;
I spent some time here to ensure that the assumed assertion "size > 0"
is always true. Well, it seems to be true indeed, but the code now
looks more fragile than before.
> @@ -471,27 +473,19 @@ string_quote(const char *instr, char *outstr, int len,
> int size)
> for (i = 0; i < size; ++i) {
> c = ustr[i];
> /* Check for NUL-terminated string. */
> - if (len < 0) {
> - if (c == '\0')
> - break;
> - /* Quote at most size - 1 bytes. */
> - if (i == size - 1)
> - continue;
> - }
> - sprintf(s, "\\x%02x", c);
> - s += 4;
> + if (c == eol)
> + goto asciz_ended;
I'd rather replaced "goto asciz_ended" with "break" here
> + *s++ = '\\';
> + *s++ = 'x';
> + *s++ = "0123456789abcdef"[c >> 4];
> + *s++ = "0123456789abcdef"[c & 0xf];
> }
> } else {
> for (i = 0; i < size; ++i) {
> c = ustr[i];
> /* Check for NUL-terminated string. */
> - if (len < 0) {
> - if (c == '\0')
> - break;
> - /* Quote at most size - 1 bytes. */
> - if (i == size - 1)
> - continue;
> - }
> + if (c == eol)
> + goto asciz_ended;
and here,
> @@ -536,8 +542,21 @@ string_quote(const char *instr, char *outstr, int len,
> int size)
> *s++ = '\"';
> *s = '\0';
>
> - /* Return nonzero if the string was unterminated. */
> - return i == size;
> + /* Return zero if we printed entire ASCIZ string (didn't truncate it) */
> + if (len < 0 && ustr[i] == '\0') {
and replaced this condition with
if (len < 0 && (i < size || ustr[i] == '\0'))
so the could would look more compact and readable.
> + /* We didn't see NUL yet (otherwise we'd jump to 'asciz_ended')
> + * but next char is NUL.
> + */
> + return 0;
> + }
> +
> + return 1;
> +
> + asciz_ended:
> + *s++ = '\"';
> + *s = '\0';
> + /* Return zero: we printed entire ASCIZ string (didn't truncate it) */
> + return 0;
> }
--
ldv
pgp0DBubVC4ZG.pgp
Description: PGP signature
------------------------------------------------------------------------------ Special Offer -- Download ArcSight Logger for FREE! Finally, a world-class log management solution at an even better price-free! And you'll get a free "Love Thy Logs" t-shirt when you download Logger. Secure your free ArcSight Logger TODAY! http://p.sf.net/sfu/arcsisghtdev2dev
_______________________________________________ Strace-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/strace-devel
