Question

So I've got some C code:

#include <stdio.h>
#include <string.h>

/* putting one of the "char*"s here causes a segfault */
void main() {
  char* path = "/temp";
  char* temp;
  strcpy(temp, path);
}

This compiles, runs, and behaves as it looks. However, if one or both of the character pointers is declared as global variable, strcpy results in a segmentation fault. Why does this happen? Evidently there's an error in my understanding of scope.

Was it helpful?

Solution

As other posters mentioned, the root of the problem is that temp is uninitialized. When declared as an automatic variable on the stack it will contain whatever garbage happens to be in that memory location. Apparently for the compiler+CPU+OS you are running, the garbage at that location is a valid pointer. The strcpy "succeeds" in that it does not segfault, but really it copied a string to some arbitrary location elsewhere in memory. This kind of memory corruption problem strikes fear into the hearts of C programmers everywhere as it is extraordinarily difficult to debug.

When you move the temp variable declaration to global scope, it is placed in the BSS section and automatically zeroed. Attempts to dereference *temp then result in a segfault.

When you move *path to global scope, then *temp moves up one location on the stack. The garbage at that location is apparently not a valid pointer, and so dereferencing *temp results in a segfault.

OTHER TIPS

The temp variable doesn't point to any storage (memory) and it is uninitialized.

if temp is declared as char temp[32]; then the code would work no matter where it is declared. However, there are other problems with declaring temp with a fixed size like that, but that is a question for another day.

Now, why does it crash when declared globally and not locally. Luck...

When declared locally, the value of temp is coming from what ever value might be on the stack at that time. It is luck that it points to an address that doesn't cause a crash. However, it is trashing memory used by someone else.

When declared globally, on most processors these variables will be stored in data segments that will use demand zero pages. Thus char *temp appears as if it was declared char *temp=0.

You forgot to allocate and initialize temp:

temp = (char *)malloc(TEMP_SIZE);

Just make sure TEMP_SIZE is big enough. You can also calculate this at run-time, then make sure the size is enough (should be at least strlen(path))

As mentioned above, you forgot to allocate space for temp. I prefer strdup to malloc+strcpy. It does what you want to do.

No - this doesn't work regardless of the variables - it just looks like it did because you got (un)lucky. You need to allocate space to store the contents of the string, rather than leave the variable uninitialised.

Uninitialised variables on the stack are going to be pointing at pretty much random locations of memory. If these addresses happen to be valid, your code will trample all over whatever was there, but you won't get an error (but may get nasty memory corruption related bugs elsewhere in your code).

Globals consistently fail because they usually get set to specific patterns that point to unmapped memory. Attempting to dereference these gives you an segfault immediately (which is better - leaving it to later makes the bug very hard to track down).

I'd like to rewrite first Adam's fragment as

// Make temp a static array of 256 chars
char temp[256];
strncpy(temp, sizeof(temp), path);
temp[sizeof(temp)-1] = '\0';

That way you:

1. don't have magic numbers laced through the code, and
2. you guarantee that your string is null terminated.

The second point is at the loss of the last char of your source string if it is >=256 characters long.

The important part to note:
destination string dest must be large enough to receive the copy.
In your situation temp has no memory allocated to copy into.

Copied from the man page of strcpy:

DESCRIPTION
   The  strcpy()  function  copies the string pointed to by src (including
   the terminating '\0' character) to the array pointed to by  dest.   The
   strings  may not overlap, and the destination string dest must be large
   enough to receive the copy.

You're invoking undefined behavior, since you're not initializing the temp variable. It points to a random location in memory, so your program may work, but most likely it will segfault. You need to have your destination string be an array, or have it point to dynamic memory:

// Make temp a static array of 256 chars
char temp[256];
strncpy(temp, 256, path);

// Or, use dynamic memory
char *temp = (char *)malloc(256);
strncpy(temp, 256, path);

Also, use strncpy() instead of strcpy() to avoid buffer overruns.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top