CODEWITHSUNDEEP
cstructreturn
Boris Salimov
Boris Salimov

Reputation: 693

Segmentation fault after returning a pointer to a struct

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?

Upvotes: 3

Views: 1744

Answers (3)

Sourav Ghosh
Sourav Ghosh

Reputation: 134396

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()).

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409472

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:

  1. Actually return a structure, by value, and not a pointer.
  2. 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.)

Upvotes: 4

Bathsheba
Bathsheba

Reputation: 234875

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.

Upvotes: 3

Related Questions