Segmentation fault after returning a pointer to a struct

Question

There is program for reading from file and return a struct.

struct ion_bin
{
    int freq;
    long height;
    int amplitude;
};

//Dynamic auto allocating array
typedef struct {
  struct ion_bin *array;
  size_t used;
  size_t size;
} Ionogram;

void freeArray(Ionogram *a); //free memory
void insertArray(Ionogram *a, struct ion_bin element); //realloc memory
void initArray(Ionogram *a, size_t initialSize); //malloc memory

Ionogram* read(int argn, char* argv[])
{
    FILE* stream;
    Ionogram ionogramObj;

    //fill ionogram from file by initArray and insertArray  
    //.....

    return &ionogramObj;
}

int main(int argn, char* argv[])
{
    Ionogram* r = read(argn, argv);

    fprintf(stderr,"Array size: %d Used %d\n",r->size, r->used); //SEGMENTATION FAULT ERROR
    //int second = (*(r->array + 2)).amplitude; //YET SEGMENTATION FAULT ERROR TOO

    //fprintf(stderr, "%d", second);
    return 0;
}

This program compile successfully, but in runtime and debug fires segmentation fault error (SIGSEGV) by attempt getting fields of returned struct (in main method) How to fix this error?

Some programmer dude · Accepted Answer · 2015-08-17 10:54:53Z

4

You make a beginners mistake, and return a pointer to a local variable. You got to remember that local variables goes out of scope once the function returns, and the pointers to it will then become invalid. Dereferencing this invalid pointer leads to undefined behavior.

Two possible solutions:

Actually return a structure, by value, and not a pointer.
Allocate memory for the structure using malloc, and return a pointer to this dynamically allocated memory.

Method one works well for smaller structures, like yours, but become inefficient for larger structures as the whole structure must be copied. (It's a shallow copy though, not a deep copy. So if you have pointers in the structure only the pointers are copied and not what they point to.)

edited Aug 17, 2015 at 10:54

answered Aug 17, 2015 at 10:48

Some programmer dude

411k36 gold badges420 silver badges655 bronze badges

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

4 Comments

Some programmer dude Over a year ago

@BlagovestBuyukliev True, but that have other downsides (reentrancy and thread-safety for example).

Bathsheba Over a year ago

@BlagovestBuyukliev: that sort of advice is a bit like telling a learner driver to brake with their left foot. Sure, it's a useful skill if you aspire to F1, but let's not complicate things at first.

Lundin Over a year ago

Third and best industry de facto standard solution: allocate the struct in the caller and pass a pointer to it.

Boris Salimov Over a year ago

Thank you! First answer is work for me! I think, that Second way not unsuitable for my situation, because I don't Know how many bytes I need to allocate before function read returned. Maybe I'm wrong.

Bathsheba · Accepted Answer · 2015-08-17 10:47:17Z

3

You are returning a pointer to a variable that goes out of scope at the end of the function.

  Ionogram ionogramObj;
  return &ionogramObj;

That's undefined behaviour in C.

As an alternative, malloc the memory for your structure in the function and return the pointer to that. Don't forget to free the pointer at some point.

answered Aug 17, 2015 at 10:47

Bathsheba

235k35 gold badges376 silver badges504 bronze badges

Comments

Sourav Ghosh · Accepted Answer · 2015-08-17 10:57:52Z

In your code, ionogramObj variable is local to the function read(). Once the function finishes the execution, there is no existence of ionogramObj, so, essentially the returned address becomes invalid in the caller (main()).

Accessing invalid address (pointer) invokes undefined behaviour. Segmentation fault is one of the side effect of UB.

To avoid this, you'll need to return an address which has a lifetime bigger than that of the called function . With the help of a pointer and dynamic memory allocation, you can achieve this.

See a pseudo code

Ionogram* read(int argn, char* argv[])
{
    FILE* stream = NULL; 
    Ionogram *ionogramObj = NULL;                 //take a pointer

    ionogramObj = malloc(sizeof(*ionogramObj));   //allocate memory dynamically

    if (!ionogramObj)                             //don't forget to check for success
      //some error message, return or exit, maybe?
    else
       //do normal operation
    //fill ionogram from file by initArray and insertArray  
    //.....

    return ionogramObj;                           //return the pointer
}

Also, dynamically allocated memory needs to be free()d to avoid memory leak. Once you're done using the return value, you can call free() with the returned pointer in the caller (main()).

Collectives™ on Stack Overflow

Segmentation fault after returning a pointer to a struct

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related