An unexpected strcpy segfault

Question

I've discovered a segfault that I'm having trouble parsing. Lest you think I haven't searched, I don't think the issue is the same as in this question. I have the following typedef'd structure:

typedef struct usage usage;
struct usage{
      char name[9]; 
      int loc;  
      usage *next;
};

I'm reading data from a file that consists of a number K followed by K pairs (S,D) where S= an 8-character string [this is a variable name] and d= an integer [a memory location].

Here's the code that's causing the error:

void addUse(int index,char *nm, int addr){
     usage *temp;
     strcpy(temp->name,nm); //segfault here. 
     temp->loc = addr;
     temp->next= NULL;
    /* more processing */
}

To make this clearer, I am calling this function from a block where I have

int dummyIndex = 1;
char s1[9];
int val1; 
scanf(" %s %d, s1, &val1);
addUse(dummyIndex, s1, val1);

It seems like in the question I linked to the issue is that they do not allocation the char on the heap. I am not sure what's happening here. Using identical calls to strcpy on another struct with a field char name[9] works just fine.

What am I missing? What have I over looked?

Thanks in advance!

score 1 · Answer 1 · answered Feb 25 '16 at 18:28

1

You've forgotten to initialize the temp pointer, so it's pointing to random memory. When you then write

strcpy(temp->name, mm);

You're following a pointer to a random address and writing bytes there, hence the segfault.

answered Feb 25 '16 at 18:28

templatetypedef

362,284
104
897
1,065

hey! thanks! i saw you posted your answer as I was typing my solution. staring at code for days can create silly errors. – jBannon Feb 25 '16 at 18:30

score 1 · Answer 2 · edited Feb 25 '16 at 18:48

1

The problem is in the following line of your code

usage *temp;
strcpy(temp->name,nm); //segfault here.

You are not initializing the temp pointer so it is taking a garbage value.

Try initializing it before strcpy

usage *temp = malloc(sizeof(usage));

Hope this helps.

edited Feb 25 '16 at 18:48

David Ranieri

39,972
7
52
94

answered Feb 25 '16 at 18:29

Sudipta Kumar Sahoo

1,049
8
16

There is no `new` in `C`, you mean `malloc(sizeof *temp);` ;) – David Ranieri Feb 25 '16 at 18:39
Oh, I thought it is C++. You are right. I mean malloc. – Sudipta Kumar Sahoo Feb 25 '16 at 18:40
@AlterMann I am sure (and tested), you cannot assign void* to usage*. So it is mandatory to type cast it like this. usage* temp = (usage*) malloc(sizeof(usage)); – Sudipta Kumar Sahoo Feb 25 '16 at 18:52
@AlterMann please edit the answer, otherwise it makes the answer incorrect. – Sudipta Kumar Sahoo Feb 25 '16 at 18:55
Are you using a C++ compiler? you should [not cast the result of `malloc` in `C`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – David Ranieri Feb 25 '16 at 18:56

score 0 · Answer 3 · answered Feb 25 '16 at 18:28

0

Here's my issue:

in other calls I make sure to set temp = malloc(sizeof(struct)); not in this one so I'm trying to write to memory that hasn't been allocated!.

answered Feb 25 '16 at 18:28

jBannon

9
2

Also better to use `%8s` instead of `%s` in `scanf`. – ameyCU Feb 25 '16 at 18:32

score 0 · Answer 4 · answered Feb 25 '16 at 18:39

The problem with your code is clear, and the SegFault happens in the exact location where it should. The reason is that you are using memory you haven't previously allocated.

void addUse(int index,char *nm, int addr){
     usage *temp;
     strcpy(temp->name,nm); //segfault here. 
     // ...
}

When you declare "usage *temp;" you're creating a pointer temp of type usage. Terrific. But... where is the pointer pointing to? You don't know. The value of temp, the memory address it contains is the garbage that happens to be in the place it occupies in memory when it is created (other programming languages give a default value to uninitialized variables, but C is not in that group).

So... you're being lucky. Your program is best described as "undefined behaviour", since it is possible that temp contains a memory address that casually is unoccupied: your program could run pass this function, crash elsewhere, and you would not have a clue about why.

Another problem is that you should return the new struct usage, while your function is returning void.

This is the function as it should be:

usage * addUse(int index,char *nm, int addr){
     usage *temp = malloc(sizeof(usage));

     if ( temp == NULL) {
         fprintf(stderr, "Not enough memory");
         exit(EXIT_FAILURE);
     }

     strcpy(temp->name,nm); //segfault here. 
     temp->loc = addr;
     temp->next= NULL;
    /* more processing */

     return temp;
}

I understand that you should link the pointer returned by addUse() to a linked list.

Hope this helps.

An unexpected strcpy segfault

4 Answers4