Re: strcat+strcat+strcat == baaad

[Shachar Shemesh]

When I'm wrong, I'm wrong.

sun@sun:~/sources/wine/test$ gcc -O0 strcpy.c -o way1 -DWAY1
sun@sun:~/sources/wine/test$ gcc -O0 strcpy.c -o way2 -DWAY2
sun@sun:~/sources/wine/test$ gcc -O0 strcpy.c -o way3 -DWAY3
sun@sun:~/sources/wine/test$ ./way1
Operation took 0 seconds 450763 usec
sun@sun:~/sources/wine/test$ ./way2
Operation took 0 seconds 598408 usec
sun@sun:~/sources/wine/test$ ./way3
Operation took 0 seconds 427037 usec

With higher O values, the difference becomes bigger, but I'm not sure then that some of the operations are not optimized out of the process, which makes the entire benchmark useless.

So, what are our conclusions? Do we just implement strlcpy and strlcat so that everyone has a function that is both efficient AND secure?

Do we go for David's suggestion, that is more efficient, but is also more cubersome and requires two extra vars to implement right?

Shachar

David Laight wrote:

I said that due to the wall to wall agreement over the superiority of sprintf/snprintf. If there is a consensus, we should stick to it.

But the performance of it will suck badly.....

Something like:

char *
str_add(char *s, char *lim, const char *a)
{
int c;

do {
if (s >= lim) {
s = lim - 1;
c = 0;
} else
c = *a++;
*s++ = c;
} while (c);

return s;
}

So you can do:
lim = buf + len;
buf = str_add(buf, lim, "abc");
buf = str_add(buf, lim, "123");
...
might be safer.

David

#include <stdio.h>
#include <string.h>
#include <sys/time.h>
#include <time.h>

const int NUM_ITER=100000;
const char * const STRING="01234567890123456789012345678901234567890123456789";

char *
str_add(char *s, char *lim, const char *a)
{
	int c;

	do {
		if (s >= lim) {
			s = lim - 1;
			c = 0;
		} else
			c = *a++;
		*s++ = c;
	} while (c);

	return s;
}

int main()
{
    struct timeval before, after;
    char buffer[1000];
    int i;

    gettimeofday(&before, NULL);
    for( i=0; i<NUM_ITER; ++i )
    {
#ifdef WAY1
	strcpy( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
	strcat( buffer, STRING );
#elif WAY2
	sprintf( buffer, "%s%s%s%s%s%s%s%s%s%s", STRING, STRING, STRING, STRING,
		STRING, STRING, STRING, STRING, STRING, STRING );
#elif WAY3
	char *pointer=buffer;
	char *const limit=buffer+sizeof(buffer);

	buffer[0]='\0';
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
	pointer=str_add( buffer, limit, STRING );
#endif
    }
    gettimeofday(&after, NULL);

    {
	long diff_sec, diff_usec;
	diff_sec=after.tv_sec-before.tv_sec;
	diff_usec=after.tv_usec-before.tv_usec;

	if( diff_usec<0 )
	{
	    diff_usec+=1000000;
	    diff_sec--;
	}

	printf("Operation took %ld seconds %ld usec\n", diff_sec, diff_usec );
    }

    return 0;
}