[PATCH] registry: Make RegisterExtensionNames slightly less inefficient

[Aaron Plattner]

On 11/13/2013 11:06 AM, Adam Jackson wrote:
> The big change is to defer strdup until we actually know the line
> matches the extension we're initializing.  This cuts the number of calls
> to strdup on this path from 24090 to 554, and 'Xvfb -pogo' drops from
> about 45M to 38M instructions retired according to valgrind.
>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>   dix/registry.c | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/dix/registry.c b/dix/registry.c
> index 82a3340..52ac9da 100644
> --- a/dix/registry.c
> +++ b/dix/registry.c
> @@ -124,10 +124,13 @@ RegisterExtensionNames(ExtensionEntry * extEntry)
>   {
>       char buf[256], *lineobj, *ptr;
>       unsigned offset;
> +    int namelen;
>
>       if (fh == NULL)
>           return;
>
> +    namelen = strlen(extEntry->name);
> +
>       rewind(fh);
>
>       while (fgets(buf, sizeof(buf), fh)) {
> @@ -149,26 +152,22 @@ RegisterExtensionNames(ExtensionEntry * extEntry)
>               goto invalid;
>           }
>
> -        /* Check for space character in the fifth position */
> -        ptr = strchr(buf, ' ');
> -        if (!ptr || ptr != buf + 4)
> +        /* check for proper formatting */
> +        if (buf[4] != ' ')

We should probably memset buf or let the C compiler initialize it, or 
this could dereference uninitialized memory if there's a line that's 
only 1, 2, or 3 chars long in fh.

>               goto invalid;
>
> +        /* check for extension name match */
> +        if (strncmp(buf + 5, extEntry->name, namelen))
> +            goto skip;
> +
> +        if (buf[5 + namelen] != ':')
> +            goto skip;
> +
>           /* Duplicate the string after the space */
> -        lineobj = strdup(ptr + 1);
> +        lineobj = strdup(buf + 5);
>           if (!lineobj)
>               continue;
>
> -        /* Check for a colon somewhere on the line */
> -        ptr = strchr(buf, ':');
> -        if (!ptr)
> -            goto invalid;
> -
> -        /* Compare the part before colon with the target extension name */
> -        *ptr = 0;
> -        if (strcmp(buf + 5, extEntry->name))
> -            goto skip;
> -
>           /* Get the opcode for the request, event, or error */
>           offset = strtol(buf + 1, &ptr, 10);
>           if (offset == 0 && ptr == buf + 1)
>

-- 
Aaron