strcpy and strcat cause problems sometimes

Question

hello I have a code like the one below

char *str ;

        strcpy(str, "\t<");
        strcat(str, time);
        strcat(str, ">[");
        strcat(str, user);
        strcat(str, "]");
        strcat(str, "(");
        strcat(str, baseName);
        strcat(str, ") $ ");

        printf("\String is now: %s\n", str);

This code seems working but when I use XCode analyse function, it says "Function call argument is an uninitialized value" and also it sometimes causes my program crash.. when I remove it, then it works fine... Whats wrong with that? Thanks

Serilda

have you allocated memory for str? – Louettalough

@kanoz: It's important because you can't write into anything except legally allocated memory. – Negligee

For future reference, str is commonly known as a dangling or wild pointer: en.wikipedia.org/wiki/Dangling_pointer – Griswold

Answer 1

strcpy and strcat are used to copy and concatenate strings to an allocated char array.

Since str in not initilized you're writing somewhere in memory and this is bad because you're corrupting other data. It may work at that moment but sooner or later you'll program will crash.

You should allocate memory when declaring str:

char str[100];

Also, strcat is not efficient as it needs to search for the string end to know where concatenate chars. Using sprintf would be more efficient:

sprintf(str, "\t<%s>[%s](%s) $ ", time, user, baseName);

Finally, if you can't guarantee the generated string will fit the array, you'd better use snsprintf.

Keyway

Answer 2

You don't allocate memory and you leave str uninitialized. All later writes are done through an uninitialized pointer that points "somewhere" - that's undefined behavior.

You have to allocate (and later free) memory large enough to hold the resulting string:

char *str = malloc( computeResultSizeSomehow() );
if( str == 0 ) {
   // malloc failed - handle as fatal error
}

//proceed with your code, then

free( str );

Negligee

what is computeResultSizeSomehow()? – Serilda

Try reading something about pointers before trying to use them. boredzo.org/pointers – Louettalough

@kanoz: You have to evaluate how much memory you need. That's called pseudocode. – Negligee

Answer 3

This is much simpler and error-free from buffer overflows:

#define BUFFERSIZE 512
char str[BUFFERSIZE];

snprintf(str, BUFFERSIZE, "\t<%s>[%s](%s) $ ", time, user, baseName);

Chorister

sprintf "prints" in a string, not in your monitor. – Louettalough

@Chorister You mean the fixed-size array? Or the snprintf? Cause the first only constraints you. – Louettalough

If you need it somewhere else, for example by returning it from the function: return strdup(str), and free it later. – Chorister

sprintf prints in an array of characters in your "storage", not to your stdout(1)->output screen – Maller