Segmentation fault

[JS]

I have this struct:

struct list {
char *thread;
struct list *previous;
struct list *next;
};

struct list *n;



I then have this function:


void addthread(char *t){
n = (struct list *)malloc(sizeof(struct list));
printf("%s\n",t);


strcpy(n->thread,t);

}

In my main function I give the addthread function this argument:

addthread("test");

when I run the program it prints "test" from line 2 in the addthread
function but then I gives the error:

Segmentation fault

I guess it has something to do with this statment:

strcpy(n->thread,t);

But why is that statement illegal?

 

[lndresnick]

I have this struct:

struct list {
char *thread;
struct list *previous;
struct list *next;
};

struct list *n;



I then have this function:


void addthread(char *t){
n = (struct list *)malloc(sizeof(struct list));
printf("%s\n",t);


strcpy(n->thread,t);

}

In my main function I give the addthread function this argument:

addthread("test");

when I run the program it prints "test" from line 2 in the addthread
function but then I gives the error:

Segmentation fault

I guess it has something to do with this statment:

strcpy(n->thread,t);

But why is that statement illegal?

n->thread is a char * pointing nowhere in particular in your code, it
has
not been initialized. You can't copy bytes to the location pointed to
by it unless you are very (un)lucky. You need something like this (and
BTW you should always check that malloc doesn't return NULL, I'll leave
that to you):

size_t len = strlen(t) + 1;
n->thread = malloc(len);
memcpy(n->thread, t, len);

-David

 

[jim1154]

I guess it has something to do with this statment:

strcpy(n->thread,t);

You need to allocate memory for the string before you attempt to copy
it. Adding the following line above your strcpy line should do it:

n->thread = (char *)malloc(strlen(t) + 1);

 

[Default User]

You need to allocate memory for the string before you attempt to copy
it. Adding the following line above your strcpy line should do it:

n->thread = (char *)malloc(strlen(t) + 1);

The cast for malloc() is unnecessary in C and can mask a dangerous
problem when there is no declaration in scope for malloc().



Brian

 

[Jason Curl]

I have this struct:

struct list {
char *thread;
struct list *previous;
struct list *next;
};

struct list *n;



I then have this function:


void addthread(char *t){
n = (struct list *)malloc(sizeof(struct list));

At this point here, the values of 'thread', 'previous' and 'next' are
all undefined.

printf("%s\n",t);


strcpy(n->thread,t);

So now you try to copy the buffer pointed to by 't' to the memory
pointed to by 'thread'. But wait - thread has not been defined.

You need to do something like:

n->thread = malloc(strlen(t)+1);
strcpy(n->thread, t);

Make sure you don't forget to free() the memory when you've finished!

free(n->thread);
free(n);

Also, you would probably want to define 'next' and 'previous':

n->previous = NULL;
n->next = NULL;

}

In my main function I give the addthread function this argument:

addthread("test");

when I run the program it prints "test" from line 2 in the addthread
function but then I gives the error:

Segmentation fault

I guess it has something to do with this statment:

strcpy(n->thread,t);

You're right, you're using an undefined pointer.

But why is that statement illegal?

See above.

 

[JS]

At this point here, the values of 'thread', 'previous' and 'next' are
all undefined.


So now you try to copy the buffer pointed to by 't' to the memory
pointed to by 'thread'. But wait - thread has not been defined.

You need to do something like:

n->thread = malloc(strlen(t)+1);

I presume you add 1 because strlen only takes the size of a string excluded
the NULL character.

strcpy(n->thread, t);

Make sure you don't forget to free() the memory when you've finished!

free(n->thread);
free(n);

Why do I need to free n->thread and n if I later on would like to use them?
The idea is to store a whole bunch of info in a list and then it should be
possible to access that info afterwards.

 

[Keith Thompson]

Jason Curl wrote: [...]

You need to do something like:

n->thread = malloc(strlen(t)+1);

I presume you add 1 because strlen only takes the size of a string excluded
the NULL character.
Yes.

strcpy(n->thread, t);

Make sure you don't forget to free() the memory when you've finished!

free(n->thread);
free(n);

Why do I need to free n->thread and n if I later on would like to use them?
The idea is to store a whole bunch of info in a list and then it should be
possible to access that info afterwards.

"Make sure you don't forget to free() the memory *when you've
finished*!" He meant when you're finished using them, not when you're
finished setting them up.

 

[JS]

At this point here, the values of 'thread', 'previous' and 'next' are
all undefined.


So now you try to copy the buffer pointed to by 't' to the memory
pointed to by 'thread'. But wait - thread has not been defined.

You need to do something like:

n->thread = malloc(strlen(t)+1);
strcpy(n->thread, t);

if the thread field in my list struct had just been:

int thread;

did I also had to do the malloc thing?

 

[JS]

Jason Curl wrote: [...]

You need to do something like:

n->thread = malloc(strlen(t)+1);

I presume you add 1 because strlen only takes the size of a string
excluded the NULL character.
Yes.

strcpy(n->thread, t);

Make sure you don't forget to free() the memory when you've finished!

free(n->thread);
free(n);

Why do I need to free n->thread and n if I later on would like to use
them? The idea is to store a whole bunch of info in a list and then it
should be possible to access that info afterwards.

"Make sure you don't forget to free() the memory *when you've
finished*!" He meant when you're finished using them, not when you're
finished setting them up.

ok, thanks.

 

[Resembelkanix]

Jason Curl wrote:





if the thread field in my list struct had just been:

int thread;

did I also had to do the malloc thing?

Not if its just a normal int.