2

In C89, the following code works fine :

typedef struct{
    int size;
    int **ptr;
}foo;

int main()
{
    foo *var;

    var = malloc(sizeof(foo) + 2 * sizeof(int*));

    int a = 3, b = 6;

    var->ptr[0] = &a;
    var->ptr[1] = &b;

    printf("%d %d\n", (*var->ptr[0]), (*var->ptr[1]));

    return 0;
}

When executed it shows : 3 6

But if I replace

var = malloc(sizeof(foo) + 2 * sizeof(int*));

with

var = malloc(sizeof(foo));
var = realloc(var, sizeof(foo) + 2 * sizeof(int*));

It compiles without any warning but I get a segmentation fault at the following line :

var->ptr[0] = &a;

I really have no idea what I am doing wrong so I would appreciate any input. I'm a beginner so I apologize if my mistake is obvious.

Thank you.

9
  • 1
    You never initialize the ptr field. Commented Jan 23, 2018 at 22:40
  • @DevSolar: Well, actually he DOES allocate memory for ptr (that is what the + 2 * sizeof(int*) is for), he just doesn't assign ptr to point at that memory Commented Jan 23, 2018 at 22:41
  • 2
    both versions are wrong, the first one exhibited a particularly bad form of Undefined Behaviour - it appeared to work fine Commented Jan 23, 2018 at 22:41
  • what exactly are you trying to do? Do you want a single continuous block of memory for that struct, including the array of ints. Or do you just want that struct pointing to the array Commented Jan 23, 2018 at 22:48
  • I wonder why newbies post their questions and then don't stick around to answer any questions that come up by people trying to help them. Commented Jan 23, 2018 at 22:50

5 Answers 5

5

I think your declarations are wrong. The following line assumes that ptr points to some memory when in fact it has not been initialized at all!

var->ptr[0] = &a;

If your compiler supports it, shouldn't it be more like this instead:

typedef struct{
    int size;
    int* ptr[0];
}foo;

Otherwise, change ptr[0] to ptr[1], and adjust your calculations based on the fact that the structure already includes one element.

Note: Once you write to a wrong address, the result is undefined. You have a problem there but it only happens to show up in one case. The problem is still there in both cases.

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

2 Comments

you should note that his code is storing the address of a not the value of a.
@pm100: Right you are.
2

I would say this is also wrong in C89.

malloc(sizeof(foo) + 2 * sizeof(int*));

is allocating more than you need it, but not in the way you expect.

+-------+------------+---------+--------+
|  size | ptr        |  int*   | int*   |
+-------+------------+---------+--------+

|  sizeof foo        | 2 * sizeof(int*) |

As you see, var->foo is still uninitialized, no matter how many extra bytes you've allocated, so

var->ptr[0] = &a;
var->ptr[1] = &b;

is going to fail.

What you should have done is:

var = malloc(sizeof *var);

var->ptr = malloc(2 * sizeof *var->ptr);

var->ptr[0] = &a;
var->ptr[1] = &b;

Then it would look like this

                                     | int |     | int |

                                     +-----+     +-----+
                                     |  a  |     |  b  |
                                     +-----+     +-----+
                                         ^         ^
                                         |         |
                                         |         |
+-------+------------+  points to     +---------+--------+
|  size | ptr        | ------------>  | int*    | int*   |
+-------+------------+                +---------+--------+

|  sizeof foo        |                | 2 * sizeof(int*) |

Don't forget to check for the return value of malloc and to free the memory afterwards.

edit

what would work is:

var = malloc(sizeof(foo) + 2 * sizeof(int*));
var->ptr = (int**) ( ((char*) var) + sizeof *var );

where you set var->pointer to point past the memory for the struct. While this might work, I think it's pretty ugly, I would reject a commit of a co-worker with something like this.

1 Comment

Thanks, very clear answer. For some reason, I thought I couldn't directly use malloc on a variable inside a struct but I was obviously mistaken. Thanks to everyone who answered, many answers were also useful although some didn't seem to realize that I wanted to store the adresses of a and b and not their values.
1

The problem is that you are not assigning ptr to point at anything, so it points to random memory, and any access of ptr[index] is undefined behavior.

Changing the declaration of ptr like Jonathan's answer suggested is the safest option, eg:

typedef struct{
    int size;
    int* ptr[1]; // or 0 if your compiler supports it
}foo;

int main()
{
    foo *var;

    var = malloc(offsetof(foo, ptr));
    if (var) {
        var->size = 0;
        ...
    }

    foo *newvar = realloc(var, offsetof(foo, ptr) + (2 * sizeof(int*)));
    if (newvar) {
        var = newvar;
        var->size = 2;

        int a = 3, b = 6;

        var->ptr[0] = &a; // <-- OK!
        var->ptr[1] = &b; // <-- OK!

        printf("%d %d\n", *(var->ptr[0]), *(var->ptr[1]));
    }

    free(var);
    return 0;
}

But, if for whatever reason, you can't change the declaration of ptr (foo is used with an existing API, for instance), then you can do this instead:

typedef struct{
    int size;
    int **ptr;
}foo;

int main()
{
    foo *var = malloc(sizeof(foo));
    if (var) {
        var->size = 0;
        var->ptr = NULL; // <-- add this!
        ...
    }

    foo *newvar = realloc(var, sizeof(foo) + (2 * sizeof(int*)));
    if (newvar) {
        var = newvar;
        var->size = 2;
        var->ptr = (int**)(var + 1); // <-- add this!

        int a = 3, b = 6;

        var->ptr[0] = &a; // <-- OK!
        var->ptr[1] = &b; // <-- OK!

        printf("%d %d\n", *(var->ptr[0]), *(var->ptr[1]));
    }

    free(var);
    return 0;
}

In this latter case, make sure you do the same ptr assignment every time you want to (re)allocate var. I would wrap that logic in a set of helper functions, eg:

typedef struct{
    int size;
    int **ptr;
}foo;

foo* createFoo(int size)
{
    foo *var = malloc(sizeof(foo) + (size * sizeof(int*)));
    if (var) {
        var->size = size;
        var->ptr = (int**)(var + 1);
    }
    return var;
}

foo* resizeFoo(foo **var, int newSize)
{
    foo *newvar = realloc(*var, sizeof(foo) + (newSize * sizeof(int*)));
    if (newvar) {
        newvar->size = newSize;
        newvar->ptr = (int**)(newvar + 1);
        *var = newvar;
    }
    return newvar;
}

int main()
{
    foo *var = createFoo(0);
    ...

    if (resizeFoo(&var, 2)) {
        int a = 3, b = 6;

        var->ptr[0] = &a; // <-- OK!
        var->ptr[1] = &b; // <-- OK!

        printf("%d %d\n", *(var->ptr[0]), *(var->ptr[1]));
    }

    free(var);
    return 0;
}

Comments

0

You need to fix the struct:

typedef struct{
    int size;
    int *ptr;
}foo;

And the code:

 var = malloc(sizeof(foo));
 var->ptr = malloc(2 * sizeof(int));

 int a = 3, b = 6;
 // store a and b in array, not their addresses
 var->ptr[0] = a;
 var->ptr[1] = b;

Assuming you want 2 entries in the array. And of course, ignoring the fact that I am not testing the result of malloc().

If you want to resize the array, just realloc var->ptr instead of var.

2 Comments

because he doesnt know what he's doing - we should ask him :-)
i doubt you did that , he is storing the address of local variables in that struct. I asked - lets see what he says
0

In Visual Studio, Windows 10, both of your examples doesn't work. This is because the struct is uninitialized in both situations.

The arrow operation:

foo->bar

is a shorthand write for:

(*foo).bar

Therefore, if "foo" struct variable is uninitialized, depending on the compiler, the pointer inside your struct will point to a random location in memory.

So, when you used the arrow operator as such:

var->ptr[0] = &a;

What you did was:

(*var).ptr[0] = &a;

So, basically, you tried to write the address of "a", into a location in memory, that you shouldn't have accessed. This is why you are getting a segmentation fault.

However, if you instead have initialized it:

int goodmemorylocation = 0;
var->ptr = &goodmemorylocation; // You change the location of your pointer to an accessible location in memory

Then your code works for both cases, in my system.

Perhaps, in your system, the first example that you used malloc, somehow points to a correct address in your memory. This is why it works.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.