Segmentation fault while using strcpy()?

Question

I have a global structure:

struct thread_data{
   char *incall[10];
   int syscall arg_no;
   int client_socket;
};

and in main()

char buffer[256];
char *incall[10];
struct thread_data arg_to_thread;

strcpy(incall[0],buffer);   /*works fine*/
strcpy(arg_to_thread.incall[0],buffer); /*causes segmentation fault*/

Why does this happen and Please suggest a way out.

thanks

Madriene

where are you allocating memory to store those strings? – Angelicangelica

won't it automatically store in stack – Madriene

no, all that's allocated is 10 pointers. – Angelicangelica

Answer 1

A segfault means that something is wrong. But no segfault does not mean that something isn't wrong. If two situations are basically the same, and one segfaults and the other does not, it usually means that they are both wrong, but only one of them happens to be triggering the segfault.

Looking at the line char* incall[10], what that means is you have an array of 10 pointers to a char. By default, these pointers will be pointing at random places. Therefore, strcpying into incall[0] will be copying the string to a random location. This is most likely going to segfault! You need to initialise incall[0] first (using malloc).

So a bigger question is why doesn't the first line segfault? I would imagine the reason is that it just so happens that whatever was in memory before was a valid pointer. Therefore, the strcpy doesn't segfault, it just overwrites something else which will later cause completely unexpected behaviour. So you must fix both lines of code.

Another issue (once you have fixed that) is that strcpy itself is highly dangerous -- since it copies strings until it finds a 0 byte and then stops, you can never be sure exactly how much it's going to copy (unless you use strlen to allocate the destination memory). So you should use strncpy instead, to limit the number of bytes copied to the size of the buffer.

Sagacious

'Circumspect'? Perhaps you meant 'suspect' or 'dangerous' or ... If used by someone who knows what they're doing (which means, amongst other things, how long the source string is and how much space there is in the destination buffer), strcpy() is perfectly safe. Not everyone ensures that they are safe. (There is an argument that if you know how long the strings are, you can use memmove(), or sometimes memcpy(), to do the string copy more efficiently.) Remember that strncpy() does not guarantee null termination of your strings. (Using strncat() is even more fraught with danger!) – Palm

@Jonathan Leffler This is true, but I'm giving advice to someone who doesn't know what they're doing (and I did say "unless you use strlen to allocate the destination memory"). But that is a very good point about strncpy -- I'm not sure what is worse: overwriting the buffer, or having a non-null-terminated string. (I would still say the former is worse, so prefer strncpy over strcpy.) – Sagacious

It's tricky. The standard C string functions leave much to be desired (but they are standard, widely available, and can be used safely with care). The lack of null termination on a short copy is so serious that the other benefits of strncpy() are, IMNSHO, outweighed by the poor behaviour in the one case where you really need it to behave sensibly. I'd live with its zero padding if it guaranteed null termination. In its original application, not null terminating was a positive virtue, but that is ancient history now. (It was designed to copy 14 character file names in directories on Unix.) – Palm

Answer 2

You've not initialized the pointer incall[0], so goodness only knows where the first strcpy() writes to. You are unlucky that your program does not crash immediately.

You've not initialized the pointer arg_to_thread.incall[0], so goodness only knows where the second strcpy() writes to. You are lucky that your program crashes now, rather than later.

In neither case is it the compiler's fault; you must always ensure you initialize your pointers.

Palm

Answer 3

1. Make sure you have enough memory allocated for your string buffers.
2. Stay away from strcpy. Use strncpy instead. strcpy is a notorious source of buffer overflow vulnerabilities - a security and maintenance nightmare for which there really isn't an excuse.

Salvadorsalvadore

Remember that strncpy() does not guarantee null termination of your strings if the source is too long. It also guarantees null padding of your string if the source is shorter than the destination. (Using strncat() is even more fraught with danger! What size do you specify in the length parameter? The answer is not the size of the target string!) – Palm

My point is, relying solely on the sentinel to determine the string length is fragile. Explicitly carrying string lengths around is not, even though you can of course still produce bugs. – Salvadorsalvadore