Seg fault when using structure pointers to access struct members in C

Question

What is wrong with my program, I get seg fault when I try to print the values.

My aim is assign some values in sample_function.

and in main function I want to copy the structure to another structure.

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

typedef struct
{
    char        *name;
    char        *class;
    char        *rollno;
} test;

test *
sample_function ()
{
    test *abc;
    abc = (test *)malloc(sizeof(test));

    strcpy(abc->name,"Microsoft");
    abc->class = "MD5";
    abc->rollno = "12345";
printf("%s %s %s\n",abc->name,abc->class,abc->rollno);
return abc;

}

int main(){

test   *digest_abc = NULL;
   test   *abc = NULL;

abc = sample_function();

digest_abc = abc;
printf(" %s  %s  %s \n",digest_abc->name,digest_abc->class,digest_abc->rollno);

return 1;

}

Pointer has always been a nightmare for me, I never understood it.

You haven't allocated memory for abc => undefined behaviour. — Daniel Fischer
– Daniel Fischer, Commented Jun 14, 2013 at 15:22
You don't need to cast the return value of malloc in a C program. — Carl Norum
– Carl Norum, Commented Jun 14, 2013 at 15:28
Why you are only using strcpy for name but not class and rollno? — Ramsharan
– Ramsharan, Commented Jun 14, 2013 at 15:44

Useless · Accepted Answer · 2013-06-14 19:04:46Z

7

test * sample_function ()
{
    test *abc;

    strcpy(abc->name,"Surya");

What do you think abc points to, here? The answer is, it doesn't really point to anything. You need to initialize it to something, which in this case means allocating some memory.

So, let's fix that first issue:

test * sample_function ()
{
    test *abc = malloc(sizeof(*abc));

    strcpy(abc->name,"Surya");

Now, abc points to something, and we can store stuff in there!

But ... abc->name is a pointer too, and what do you think that points to? Again, it doesn't really point to anything, and you certainly can't assume it points somewhere you can store your string.

So, let's fix your second issue:

test * sample_function ()
{
    test *abc = malloc(sizeof(*abc));

    abc->name = strdup("Surya");
    /* ... the rest is ok ... */
    return abc;
}

Now, there's one last issue: you never release the memory you just allocated (this probably isn't an issue here, but it'd be a bug in a full-sized program).

So, at the end of main, you should have something like

    free(abc->name);
    free(abc);
    return 1;
}

The final issue is a design one: you have three pointers in your structure, and only convention to help you remember which is dynamically allocated (and must be freed) and which point to string literals (and must not be freed).

That's fine, so long as this convention is followed everywhere. As soon as you dynamically allocate class or rollno, you have a memory leak. As soon as you point name at a string literal, you'll have a crash and/or heap damage.

As japreiss points out in a comment, a good way to enforce your convention is to write dedicated functions, like:

void initialize_test(test *obj, const char *name, char *class, char *rollno) {
    obj->name = strdup(name);
    ...
}
void destroy_test(test *obj) {
    free(obj->name);
}
test *malloc_test(const char *name, ...) {
    test *obj = malloc(sizeof(*obj));
    initialize_test(obj, name, ...);
    return test;
}
void free_test(test *obj) {
    destroy_test(obj);
    free(obj);
}

edited Jun 14, 2013 at 19:04

answered Jun 14, 2013 at 15:23

Useless

68.5k6 gold badges100 silver badges143 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

android.developer Over a year ago

I have tried allocating memory by malloc, still hitting seg fault - code edited

Useless Over a year ago

congratulations, you hit the second bug while I was editing it into my answer :)

android.developer Over a year ago

That solved it thanks, one last question if I free memory allocated to abc and still want to store values in digest_abc - how do I do that

japreiss Over a year ago

+1 this kind of nested memory allocation for structs is common, and it's a good idea to wrap it in a "constructor" function like void initialize_test(test *the_test, char *name, char *class, char *rollno) that takes care of all the allocation and copying, and void cleanup_test(test *obj) to free memory.

Hunter McMillen · Accepted Answer · 2013-06-14 15:38:39Z

1

In your function sample_function you return a pointer to abc. You cannot do this in C due to the way Activation Records are organized.

An Activation Record is a data structure that contains all the relevant information for a function call, parameters, return address, addresses of local variables, etc...

When you call a function a new Activation Record gets pushed onto the stack it could look something like this.

// Record for some function f(a, b)
| local variable 1  | <- stack pointer  (abc in your case)
| local variable 2  |
| old stack pointer | <- base pointer
| return address    |   
| parameter 1       |
| parameter 2       |
---------------------
| caller activation | 
|   record          |

When you return from a function this same activation record gets popped off of the stack but what happens if you returned the address of a variable that was on the old record ?

// popped record
| local variable 1  | <- address of abc   #
| local variable 2  |                     #
| old stack pointer |                     # Unallocated memory, any new function
| return address    |                     # call could overwrite this
| parameter 1       |                     #
| parameter 2       |                     # 
--------------------- <- stack pointer 
| caller activation | 
|   record          |

Now you try to use abc and your program correctly crashes because it sees that you are accessing an area of memory that is unallocated.

You also have problems with allocation, but other answers have already covered that.

answered Jun 14, 2013 at 15:38

Hunter McMillen

61.8k25 gold badges124 silver badges176 bronze badges

7 Comments

Useless Over a year ago

The pointer test *abc is returned by value. The address of abc, which would be a test ** and would indeed be out of scope, doesn't appear anywhere I can see.

Hunter McMillen Over a year ago

@Useless I thought it was pretty common terminology, but yes stack frame could be used also.

Useless Over a year ago

I looked it up and edited that comment out before your response - it was just unfamiliar :)

Useless Over a year ago

Um, I still think the address of abc thing is a red herring though - the address of abc is never taken.

Hunter McMillen Over a year ago

@Useless Isn't that what is being returned ? I just re-read what I wrote, I meant to say address of the pointer to abc; not abc itself

|

Carl Norum · Accepted Answer · 2013-06-14 15:22:40Z

0

In sample_function you declare abc as a pointer to a test structure, but you never initialize it. It's just pointing off into the weeds somewhere. Then you try to dereference it to store values - BOOM.

Your program doesn't need any pointers at all; structures can be passed by value in C.

If you do want to keep similar interfaces to what you have now, you're going to have to add some dynamic allocations (malloc/free calls) to make sure your structures are actually allocated and that your pointers actually point to them.

answered Jun 14, 2013 at 15:22

Carl Norum

226k42 gold badges442 silver badges480 bronze badges

2 Comments

Lee Daniel Crocker Over a year ago

He's also not allocating any memory for the strings.

Carl Norum Over a year ago

Yup, it's true. That only matters for one of them, though.

Collectives™ on Stack Overflow

Seg fault when using structure pointers to access struct members in C

3 Answers 3

4 Comments

7 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

7 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related