[SCM] strace branch, master, updated. v4.6-127-g8778bff

[Dmitry V. Levin]

On Wed, Aug 31, 2011 at 10:25:34AM +0000, Denys Vlasenko wrote:
> commit 8778bffdd25dca050b3aa2a7a7e05bc8a63a6665
> Author: Denys Vlasenko <dvlasenk at redhat.com>
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20110901/879e7802/attachment.bin>