C usage of malloc on struct

Question

I'm trying to use malloc on a struct called image. The function is:

void image_init(struct image* img, int w, int h) {
    img = malloc(sizeof(struct image));
    (*img).w = w;
    (*img).h = h;
}

And the function to free the image again is:

void image_destroy(struct image* img) {
    free(img);
}

But whenever I try to free the malloc-ed data I'm getting the error that the adress I'm trying to free was not malloc-ed before.

The struct of image is:

struct image {
    int w,h;
    int* data;
}

I call the functions with:

struct image img;
image_init(&img,100,100);
image_destroy(&img);

Let's save keystrokes with one-letter struct member names and then waste two characters on (*img).w instead of img->w! Brilliant. — Kaz
– Kaz, Commented May 18, 2016 at 20:16

gmoniava · Accepted Answer · 2016-05-19 09:08:55Z

5

First here

void image_init(struct image* img, int w, int h) {
    img = malloc(sizeof(struct image));
    (*img).w = w;
    (*img).h = h;
}

img is a copy of the original pointer passed to it, hence the code on first line inside function has no effect, since it makes the copy point somewhere - not the original object.

This:

struct image img;
image_init(&img,100,100);
image_destroy(&img);

doesn't make sense either (assuming you expected img to point somewhere after call to init). img isn't a pointer, how you expect it to point somewhere?

One way you could solve this could be

struct image *img = image_init(100,100);

where

struct image* image_init(int w, int h) {
    img = malloc(sizeof(struct image));
    (*img).data = NULL;
    (*img).w = w;
    (*img).h = h;
    return img;
}

Don't forget to call free on the pointer returned by above function - you will need to free data pointer separately too, in case you allocate it too.

NOTE: My best guess (if you also can't change prototype) is you want something like this:

void image_init(struct image* img, int w, int h) {
    img->data = malloc(sizeof(int) * w * h);
    img->w = w;
    img->h = h;
}

destroy:

void image_destroy(struct image* img) {
    free(img->data);
    free(img);
}

In main

struct image* img = malloc(sizeof(struct image));
image_init(img, 100, 100);
image_destroy(img);

PS. Or if you want the usage of those functions remain as you had in main go with the answer of Johnny Mopp.

edited May 19, 2016 at 9:08

answered May 18, 2016 at 19:51

gmoniava

28.9k9 gold badges61 silver badges103 bronze badges

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

11 Comments

Maschs Over a year ago

Thanks. But the calls of image_init and the prototype of it were given to us to implement so I thought we can't change it. Are there solutions that don't change it?

Eugene Sh. Over a year ago

Or alternatively you can keep the structure similar to the original, but pass struct image **img as a parameter and assign *img with the malloced pointer.

Eugene Sh. Over a year ago

@user3745172 In this case you don't need malloc. The function should be provided with an already allocated structure pointer. As you do in the question. Just remove the malloc line and you are done.

gmoniava Over a year ago

@user3745172 In that case you might need to use pointer to pointer. Refer to google please, check how to initialize a struct using pointer to pointer.

gmoniava Over a year ago

@user3745172 see update, if you can't change prototype

|

001 · Accepted Answer · 2016-05-18 20:15:32Z

3

I think your assignment is not to allocate the img but the data ptr:

void image_init(struct image* img, int w, int h) {
    img->data = malloc(sizeof(int) * w * h);// Check for failure...
    img->w = w;
    img->h = h;
}
void image_destroy(struct image* img) {
    free(img->data);
}

answered May 18, 2016 at 20:15

001

13.6k5 gold badges38 silver badges72 bronze badges

1 Comment

Eugene Sh. Over a year ago

This makes a perfect sense.

pm100 · Accepted Answer · 2016-05-18 20:12:28Z

0

struct image img;
image_init(&img,100,100);
image_destroy(&img);

allocates an image struct on your stack. Why are you trying to malloc one? If you really want one on the heap then do

    struct image * image_init( int w, int h) {
        struct image *img = malloc(sizeof(struct image));
        img->w = w;
        img->h = h;
        return img;
    }
...  
    struct image *img = image_init(100,100);

Note the more idiomatic '->' use

if you want the one on the stack then do

void image_init(struct image* img, int w, int h) {
    img->w = w;
    img->h = h;
}

answered May 18, 2016 at 20:12

pm100

50.6k25 gold badges95 silver badges158 bronze badges

Comments

trench224 · Accepted Answer · 2016-05-18 19:53:24Z

-4

You need to cast the malloc to struct because the return value of malloc is of type void.

answered May 18, 2016 at 19:53

trench224

11 bronze badge

1 Comment

Jonathan Leffler Over a year ago

Beware of applying rules enforced by C++ compilers to C code. They're different languages, and this is one place where there's a crucial difference.

Collectives™ on Stack Overflow

C usage of malloc on struct

4 Answers 4

11 Comments

1 Comment

Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

11 Comments

1 Comment

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related