C Structs: Problem Initializing? Valgrind Errors

Question

Coding in C:

typedef struct {
   char *string;
   int one;
   int two;
} Example;

... elsewhere:

Example *new = (Example *)malloc(sizeof(Example*));
new->string = strdup(otherstring); //no problem here
new->one = 5; //valgrind says that there is an invalid write of size 4.

My program runs fine, I just can't make valgrind happy. And that probably means I will have some other error elsewhere.

I guess I don't have to declare a pointer to the structure, (i.e., I could call "Example new" instead), but I'm stuck finding out how to allocate memory on the heap because I will need to access "new" from other parts of the program.

Am I making a mistake in the above few lines? Or is my error likely to be somewhere else? This is the first memory error that valgrind reports.

EDIT: accidentally had int *s instead of ints in my struct. Fixed.

Perhaps you need `int *pi = malloc(sizeof(int)); *pi = 5; new->one = pi;` ? — Joseph Quinsey, Apr 19 '11 at 01:57

score 3 · Accepted Answer · answered Apr 19 '11 at 02:13

I see various problems here. First of all, this:

Example *new = (Example *)malloc(sizeof(Example*));

doesn't allocate the right amount of memory (and you don't need the cast). You want this:

Example *new = malloc(sizeof(Example));

Then, you say this:

new->one = 5;

and that's assigning an int to an int*; that's not a good idea and valgrind rightly complains about it. If your struct is properly declared, then you want this:

new->one = malloc(sizeof(int)); /* The other malloc doesn't allocate this, you have to. */
*(new->one) = 5;

I suspect (as you say that everything works) that you really mean to declare your struct like this:

typedef struct {
   char *string;
   int one;
   int *two;
} Example;

but there isn't enough information to be certain. Then you probably still have similar issues with new->two.

score 0 · Answer 2 · answered Apr 19 '11 at 01:55

0

Example *new = (Example )malloc(sizeof(Example));

Should be:

Example *new = (Example *)malloc(sizeof(Example);

You have to allocate the whole struct not just a reference to it. Sometimes the program runs just because you get lucky.

answered Apr 19 '11 at 01:55

Bob

573
4
9

Explicit cast, not required in C. http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Sadique Apr 19 '11 at 01:56
Just to clarify: Don't use a * inside the sizeof() – Bob Apr 19 '11 at 01:57
Bob you meant `Example *new = (Example *)malloc(sizeof(Example));`?? – Sadique Apr 19 '11 at 02:02

Marc · Answer 3 · 2011-04-19T10:42:56.910

0

Try this instead (just cut, paste, and run):

Example *new = (Example *)malloc(sizeof(Example));  //This is the correct method as pointed out by Bob, otherwise you're allocating only enough space to fit a memory location vs the struct
new->string = strdup(otherstring);  //this is okay
new->one = (int*)malloc(sizeof(int));
*(new->one) = 5; //You want to assign the value 5 to the new->one memory location, not assign new->one pointer the value of 5!!!

edited Apr 19 '11 at 10:42

answered Apr 19 '11 at 02:03

Marc

4,546
2
29
45

1

But `new->one` would need to be allocated before assigning something to where it points. – mu is too short Apr 19 '11 at 03:01
@Joseph: Sir you are absolutely correct! I've corrected my answer and +1 for you! – Marc Apr 19 '11 at 10:44

C Structs: Problem Initializing? Valgrind Errors

3 Answers3