RE: strcat+strcat+strcat == baaad

[Medland, Bill]

> -----Original Message-----
> From: [e-mail]
> [mailto:[e-mail]]On Behalf Of Francois Gouget
> Sent: Friday, November 29, 2002 10:11 PM
> To: [e-mail]
> Subject: strcat+strcat+strcat == baaad
>
> I don't like pieces of code that go:
>
> strcpy(foo, bar1);
>   strcat(foo, bar2);
>   strcat(foo, bar3);
>   strcat(foo, bar4);
>   strcat(foo, bar5);
>   strcat(foo, bar6);
>
> It's really inefficient: the cost increases quadratically 
> with the size
> of the resulting string. It's more efficient to do:
>
> sprintf(foo, "%s%s%s%s%s%s", bar1,bar2,bar3,bar4,bar5,bar6);
>
> Here the cost is proportional to the size of the resulting string.

And surely when the opportunity arises it would be better to use snprintf
(i.e. when you know the size of the buffer).  (I doubt that the code has
already checked the size of the strings being pasted together)
> For for various reasons I came accross a couple of instances of
> strcat+strcat badness and replaced them with a sprintf. So here's the
> patch.
>
> Changelog:
> -
>
> memory/environ.c,
> tools/makedep.c
>
> Use sprintf rather than calling strcat over and over
> Index: memory/environ.c
>
> RCS file: /home/wine/wine/memory/environ.c,v
> retrieving revision 1.38
> diff -u -r1.38 environ.c
> --- memory/environ.c	9 Oct 2002 18:35:02 -0000	1.38
> +++ memory/environ.c	30 Nov 2002 05:20:12 -0000
> @@ -641,12 +641,7 @@
>  /* Set the new string */
>
> - if (value)
> - {
> -
>    strcpy( p, name );
> -
>    strcat( p, "=" );
> -
>    strcat( p, value );
> - }
> - if (value) sprintf( p, "%s=%s", name, value );
> current_envdb.env = new_env;
> ret = TRUE;
> Index: tools/makedep.c
>
> RCS file: /home/wine/wine/tools/makedep.c,v
> retrieving revision 1.13
> diff -u -r1.13 makedep.c
> --- tools/makedep.c	17 Aug 2002 18:28:43 -0000	1.13
> +++ tools/makedep.c	30 Nov 2002 06:10:17 -0000
> @@ -192,9 +192,7 @@
>      if (SrcDir)
>      {
>          pFile->filename = xmalloc( strlen(SrcDir) + 
> strlen(pFile->name) + 2 );
> -
>    strcpy( pFile->filename, SrcDir );
> -
>    strcat( pFile->filename, "/" );
> -
>    strcat( pFile->filename, pFile->name );
> -
>    sprintf( pFile->filename, "%s/%s", SrcDir, pFile->name );
> }
>    else pFile->filename = xstrdup( pFile->name );
>
> @@ -218,9 +216,7 @@
>      for (path = firstPath; path; path = path->next)
>      {
>          char *filename = xmalloc(strlen(path->name) + 
> strlen(pFile->name) + 2);
> -
>    strcpy( filename, path->name );
> -
>    strcat( filename, "/" );
> -
>    strcat( filename, pFile->name );
> -
>    sprintf( filename, "%s/%s", path->name, pFile->name );
>    if ((file = fopen( filename, "r" )))
>    {
>        pFile->filename = filename;
>
>
> -- 
> Francois Gouget         [e-mail]        http://fgouget.free.fr/
>                 Linux: It is now safe to turn on your computer.