Segmentation fault occurs when accessing records in a dynamic array in the main function, but not in the included function

Question

I came across the problem where a dynamic array of structures is properly initialized and filled with data in a function from an included file, but an attempt to access the content of the array causes segmentation faults in the main function even though the pointer was defined globally. Please have a look at the example below:

my_struct.h

typedef struct my_struct {
    int one;
    int two;
} my_struct_t;

void update_my_struct(my_struct_t*, int);

my_struct.c

#include <stdlib.h>
#include <stdio.h>
#include "my_struct.h"

void
update_my_struct(my_struct_t *my_s, int num)
{
    int i;

    for (i = 0; i < num; i++ ) {
        my_s = realloc(my_s, sizeof(my_struct_t)*(i+1));
        my_s[i].one = 1*i;
        my_s[i].two = 2*i;
    }
    printf("  my_s[0] one: %d two: %d\n", my_s[0].one, my_s[0].two);
}

main.c

#include <stdlib.h>
#include <stdio.h>
#include "my_struct.h"

my_struct_t *my_structs;

void
main(void)
{
    int i;
    update_my_struct(my_structs, 4);
    for (i = 0; i < 4; i++)
        printf("  %d. one = %d, two = %d\n", i, my_structs[i].one, my_structs[i].two);
    free(my_structs);
}

Compile and run:

$ gcc main.c my_struct.c
$ ./a.out 
  my_s[0] one: 0 two: 0
Segmentation fault (core dumped)

I checked with gdb and the seg fault occurs in main, so it puzzles me how come the dynamic array can be accessed in the included function, but in the main function even though the pointer was declared in main.c. I will appreciate some helpful hints and comments on this issue.

update

Following up on EML's answer I changed the code of the update function to:

void
update_my_struct(my_struct_t *my_s, int num)
{
        int i;

        for (i = 0; i < num; i++ ) {
                if (my_s == NULL)
                        my_s = (my_struct_t *) malloc(sizeof(my_struct_t));
                else
                        my_s = (my_struct_t *) realloc(my_s, sizeof(my_struct_t)*(i+1));
                my_s[i].one = 1*i;
                my_s[i].two = 2*i;
        }
        printf("  my_s[3] one: %d two: %d\n", my_s[3].one, my_s[3].two);
}

Seg fault still occurs:

$ ./a.out 
  my_s[3] one: 3 two: 6
Segmentation fault (core dumped)

Valgrind did not provide me with any insightful information, here is the snippet of its output:

==5301== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==5301== Command: ./a.out
==5301== 
  my_s[3] one: 3 two: 6
==5301== Invalid read of size 4
==5301==    at 0x40118F: main (in a.out)
==5301==  Address 0x4 is not stack'd, malloc'd or (recently) free'd
==5301== 
==5301== 
==5301== Process terminating with default action of signal 11 (SIGSEGV): dumping core
...

Where is `update_my_struct`? If you have a bug in a function, you need to show that function in your post. — Tom Karzes, Sep 24 '20 at 08:49
You probably wrongly copied twice my_struct.h instead of popsting actual code of my_struct.c — Roberto Caboni, Sep 24 '20 at 08:50
It would be a lot easier if you posted the actual code. But even only seeing half of it, I can make a good guess as to what's going on. Most likely `update_my_struct` is allocating memory, initializing it, then returning it, without updating the global `my_structs`, which will remain `NULL`. When accessed, this results in a segmentation fault. — Tom Karzes, Sep 24 '20 at 08:54
`my_struct.c` you posted is actually `my_struct.h`. Please [edit] the question. — Jabberwocky, Sep 24 '20 at 09:09
@mac13k When you wrote "Edit made, code fixed.", did you mean you have fixed the original problem? Because the code is still broken. — Ian Abbott, Sep 24 '20 at 09:24
Your function only changes the function parameter `my_struct_t *my_s`. The global still remains with its original value (random or undefined). One way is to return the pointer from the function and assign it to the global, or pass a `my_struct_s **` to the function, and then modify the code to access the global via this pointer to pointer. It would also be good to check the return value of `realloc`. Also, `realloc` will crash because of the initial undefined value of the pointer passed to it. — DNT, Sep 24 '20 at 09:31
@DNT How a double pointer would be any better if I am dealing with a dynamic array of structs that are not dynamic? — mac13k, Sep 24 '20 at 10:27
@mak13k It's not a matter of being better or worse. It is one method of accessing your global via the pointer to pointer. You still need to initialize your global to NULL since it is not guaranteed that its initial value will be NULL. An uninitialized variable according to the standard has a `undefined` value. However, I think you need to reconsider the design, so that you keep track of how many structs you have in your array unless you only initialize it once. — DNT, Sep 24 '20 at 10:30
If you compile with `-g` (eg. `gcc -g main.c my_struct.c`), you can probably get more useful information fram `valgrind`. And you should probably also include warnings (`gcc -g -Wall -Wextra -pedantic main.c my_struct.c`), although in this case I think they wonʼt help with the issue. — Daniel H, Sep 24 '20 at 10:53
Also, that valgrind output *is* helpful: the “Address 0x4” part of “Address 0x4 is not stack'd, malloc'd or (recently) free'd” says that you are working near a NULL pointer somewhere (since 0x4 is just 4 bytes off 0x0 aka NULL). — Daniel H, Sep 24 '20 at 10:56
It is not a duplicate of https://stackoverflow.com/questions/39486797 !!! — mac13k, Sep 24 '20 at 11:11

DNT · Accepted Answer · 2020-09-24T11:28:23.280

1

Here is what I implied in my comments. This assumes *my_s is initially NULL, so realloc in that case will act like malloc.

#include <stdlib.h>
#include <stdio.h>
#include "my_struct.h"

void
update_my_struct(my_struct_t **my_s, int num)
{
    int i;
    *my_s = (my_struct_t *)realloc(*my_s, sizeof(my_struct_t)*num); 
    for (i = 0; i < num; i++ ) {
        (*my_s)[i].one = 1*i;
        (*my_s)[i].two = 2*i;
    }
    printf("  (*my_s)[0] one: %d two: %d\n", (*my_s)[0].one, (*my_s)[0].two);
}

You can then call it like this.

my_struct_t *my_structs = NULL;
...
update_my_struct(&my_structs, 4);

This will work fine for one initialization and the number of structs allocated is kept by the caller. A second call will reallocate to the new number of structs, and reinitialize everything from the beginning. I omitted realloc return value check for brevity.

And here is the full code as tested on godbolt.

  #include <stdlib.h>
  #include <stdio.h>

  typedef struct my_struct {
    int one;
    int two;
} my_struct_t;
    
    void
    update_my_struct(my_struct_t **my_s, int num)
    {
        int i;
        *my_s = (my_struct_t *)realloc(*my_s, sizeof(my_struct_t)*num); 
        for (i = 0; i < num; i++ ) {
            (*my_s)[i].one = 1*i;
            (*my_s)[i].two = 2*i;
        }
    }

    my_struct_t *my_structs = NULL;

    int main ()
    {
        int i;
        update_my_struct(&my_structs, 4);

        for (int i=0; i<4; i++)
            printf("  (*my_s)[%d] one: %d two: %d\n", i, my_structs[i].one, my_structs[i].two);
        return 0;
    }

edited Sep 24 '20 at 11:28

answered Sep 24 '20 at 10:46

DNT

2,356
14
16

Thanks, but I really need the realloc inside the loop in my real life code. Your proposed solution kind of works, but now I get seg faults in the update function after the first iteration, so it works once and fails to assign values to the struct after the second realloc. – mac13k Sep 24 '20 at 11:09
Do you pass the same global in the second call or something else? – DNT Sep 24 '20 at 11:11
Not the second call the function, but the second iteration of the for loop inside the update function - the seg fault occurs when i=1. – mac13k Sep 24 '20 at 11:12
Interestingly the seg fault occurs in the same place regardless of the realloc line being inside or outside of the loop (like in your proposed solution). – mac13k Sep 24 '20 at 11:14
I don't see how you get a segfault. This code works fine on godbolt after fixing a little typo `my_struct_s` to `my_struct_t`. – DNT Sep 24 '20 at 11:23
OK, my bad - I used `->` instead of `(* )`. It does work fine now, thanks. – mac13k Sep 24 '20 at 11:25
You got me wondering there for a second :) – DNT Sep 24 '20 at 11:26

score 0 · Answer 2 · answered Sep 24 '20 at 09:29

0

Check your realloc docs: the memory must have been previously allocated with with malloc/etc, or the results are undefined.

Your code doesn't really work either - the realloc is inside your for loop, etc.

And try running valgrind - it would have found this problem.

answered Sep 24 '20 at 09:29

EML

9,619
6
46
78

I changed it to: `malloc if NULL else realloc` also with the type cast to `my_struct_t *`, but it did not solve the problem. – mac13k Sep 24 '20 at 09:46
@mac13k: you should post your new code, and run `valgrind` if you can. – EML Sep 24 '20 at 09:49
And your suggested change won't work anyway - 'undefined behaviour' is not the same as as checking the return value for 0. Just do a `malloc` in main. – EML Sep 24 '20 at 09:55
Why can't I do the malloc in the included function? – mac13k Sep 24 '20 at 09:58
Yuo can do it wherever you want; just do a stand-alone malloc as the first step. – EML Sep 24 '20 at 10:11
But in my real code I do not know whether there are any instances of a struct to be added to the array or not, so if there is nothing the pointer should be empty which malloc will set its size to `sizeof(my_struct)` suggesting there is one element. – mac13k Sep 24 '20 at 10:17
1

Generally you can't pass an uninitialized pointer to realloc. You can however pass a NULL pointer and then it behaves just as malloc. And by accident, the OP is indeed passing a NULL pointer, since the global pointer has static storage duration. So this answer is incorrect for this specific case. – Lundin Sep 24 '20 at 10:50

Segmentation fault occurs when accessing records in a dynamic array in the main function, but not in the included function

2 Answers2