strcpy segmentation fault

Question

i have a list of quads and they have a label starting from 1. the backpatch is taking a list structure which points at some quads. i want backpatch to update those quads, putting z on the char * fourth and then emptying l so i can put other quads later.I get seg.fault in backpatch's strcpy although I have allocated memory for the char * z and char * fourth. Does anybody know why does that happens?

struct quad {
char *label; //5
char *first; //30
char *second;
char *third;
char *fourth;
struct quad *next;
};

struct list {
    struct quad *quadlist;
    struct list *nextlist;
};


void backpatch(struct list *l, char * z) {
struct list *temp = (struct list*) malloc(sizeof (struct list));
temp->nextlist = (struct list*) malloc(sizeof (struct list));
temp->quadlist = (struct quad*) malloc(sizeof (struct quad));
temp->quadlist->fourth = (char*)malloc(30 * sizeof (char));
l->nextlist = (struct list*) malloc(sizeof (struct list));
temp = l;
//z=(char*)malloc(sizeof(struct list))
while (temp->nextlist != NULL) {

    strcpy(temp->quadlist->fourth, z);
    temp = l->nextlist;
}
strcpy(temp->quadlist->fourth, z);

free(temp);
free(l);

}

even if i only keep the

while (l->nextlist != NULL) {

strcpy(l->quadlist->fourth, z);
l = l->nextlist;
}
strcpy(l->quadlist->fourth, z);
free(l);

part, its also seg.fault...

Have you double-checked that the string pointed to by `z` is not larger than your allocated 30 bytes. Also you should not cast the return of `malloc`, it's a common source of errors. — Sergey L., Apr 29 '14 at 14:15
make sure `l` is initialized properly. Cause you work on `l` after the assignment `temp=l;` — boxed__l, Apr 29 '14 at 14:19
the string pointed to by z is not larger than 30 bytes(its actually 5)... i did not exactly understand what do you mean by not casting the return of malloc :P — segmentation_fault, Apr 29 '14 at 14:20
For instance, `struct list *temp = (struct list*) malloc(sizeof (struct list));` should be just `struct list *temp = malloc(sizeof(struct list));` and similarly everywhere else you use `malloc`. — zwol, Apr 29 '14 at 14:26
[Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). — unwind, Apr 29 '14 at 14:28
i did it without casting the return value of malloc...SEG.FAULT.....:p — segmentation_fault, Apr 29 '14 at 14:56
That may mean you have neglected to `#include `, but it may also just be the same segfault as before. Also, you *have* turned warnings on in your compiler, yes? And you have corrected all the problems that this finds? — zwol, Apr 29 '14 at 15:47

score 0 · Answer 1 · answered Apr 29 '14 at 14:18

0

from the comment:

//z=(char*)malloc(sizeof(struct list))

it looks like you allocated memory for z but it's become a non-NULL terminated string. So strcpy keeps copying and eventually starts reading past the end of z. Check what is in z befor strcpy

answered Apr 29 '14 at 14:18

Serve Laurijssen

9,266
5
45
98

Jackson · Answer 2 · 2014-04-29T14:53:09.353

You create temp and allocate memory for it and its components but you the throw it away when you do temp = l; So all of these calls leak memory as you never use what you allocate or free it. The two calls to free at the end of the function are wrong, temp no longer points to the memory you allocated and you don't free the other memory allocated inside the temp structure. Freeing l destroys the head of the list your trying to update - I'm pretty sure that;s not what you intended.

When you do temp = l; you loose your refence to all the memory you just allocated, and now l and temp point to the same 'struct list'

You have struct quad *next; in the quad structure but as your list structure links the quads into a list what is this for?

Your while loop looks wrong to me - temp->nextlist is the result of yours call to malloc in l->nextlist = (struct list*) malloc(sizeof (struct list)); but you never initialise the structure so l->nextlist->nextlist will be garbage and could point anywhere.

I'd suggest that you stop looking at backpatch and write a function that displays your data, doing that will have two effects, you'll know that the structure is correct and you'll have a better understanding of how the structure links together. You need to get your head around how the structures and pointers combine to make a linked list like this. Google for C Linked List Implementation and read some existing code that implements linked lists.

What is your code actually trying to do?

i am suppose to have quads(four strings) and *l is a list of quads. struct list is a struct that has a pointer to a quad and a pointer to the rest of the list. backpatch() must enter all of the quads of the *l list and put *z into the fourth string. — segmentation_fault, Apr 29 '14 at 14:35
i ll start from the top: i have a list of quads.Thats why i need *next at quad structure. and they have a label starting from 1.Now the backpatch is taking a list structure which points at some quads. i want backpatch to update those quads, putting z on the char * fourth and then emptying l so i can put other quads later. — segmentation_fault, Apr 29 '14 at 15:11

strcpy segmentation fault

2 Answers2