C Allocating memory for array of structures containing multiple char*

Question

I have problem allocating memory for array of structure containing char*.

I have one stracture "Person"

typedef struct
{
    char *name;
    char *surname;
    char *phonenumber;
} Person;

What I want to do is to read some data from file and fill the array (Person *array) of people where I have to dynamically allocate memory.

At the moment, I have something like that:

array = malloc(sizeof(Person) * arraysize);
Person *buff;
char   text[100];
char   *result;

for(i=0; i<arraysize; i++)
{
    buff = &array[i];
    fgets(text, 100, f );

    //Read first name
    result = strtok(text,":");
    buff->name= malloc(strlen(result));
    buff->name= result;

    //Read surname
    result  = strtok(0,":");
    buff->surname = malloc(strlen(result));
    buff->surname = result;

    //Read phone number
    result = strtok(0, ":");
    buff->phonenumber = malloc(strlen(result));
    buff->phonenumber = result;
}

When I print out the whole array I don't get any valid data. I'm wondering what am I doing wrong. I appreicate for your answers in advance!

codaddict · Accepted Answer · 2012-12-14 13:15:59Z

2

The problem is:

result = strtok(text,":");
buff->name= malloc(strlen(result));
buff->name= result;

You need to use strcpy for copying and also the length of the malloced string should be one more than the string length to accommodate the NUL char.

buff->name= malloc(strlen(result) + 1);

answered Dec 14, 2012 at 13:15

codaddict

457k83 gold badges501 silver badges537 bronze badges

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

1 Comment

Blodyavenger Over a year ago

How could I forget to use strcpy <facepalm>, that makes sense now. Thanks a lot, you've saved the day :)

unwind · Accepted Answer · 2012-12-14 13:16:05Z

2

This:

buff->name= malloc(strlen(result));
buff->name= result;

is broken. You can't assign "strings" like that in C, you need to copy characters. The pointer returned by malloc() is overwritten by result, leaking memory. Also, you need to include space for the terminator.

So, this should be:

buff->name= malloc(strlen(result) + 1);
strcpy(buff->name, result);

answered Dec 14, 2012 at 13:16

unwind

402k64 gold badges492 silver badges620 bronze badges

Comments

Fred Foo · Accepted Answer · 2012-12-14 13:19:49Z

2

buff->name= result

resets the pointer buff->name to point to the same location in memory as result. If you want to copy in the string contents of result, use strcpy:

strcpy(buff->name, result);

But note that you have to reserve space for the trailing NUL character as well in the call to malloc:

buff->name = malloc(strlen(result) + 1);

If you're on a POSIX system (Linux, Mac OS X, any Unix) you can replace malloc, strlen and strcpy with the much more convenient strdup:

buff->name = strdup(result);

edited Dec 14, 2012 at 13:19

answered Dec 14, 2012 at 13:14

Fred Foo

365k80 gold badges765 silver badges852 bronze badges

2 Comments

Blodyavenger Over a year ago

Thanks, I totally forgot that I should use strcpy.

hyde Over a year ago

@Blodyavenger I think using strdup is much nicer than separate malloc + strcpy. If nothing else, it saves somebody else, who is reading the code, from wondering WTF strdup isn't used here... ;)

P.P · Accepted Answer · 2012-12-14 13:22:16Z

2

strtok overwrite the pointer everytime. So you lose the pointer to data as the iteration goes.

You can do something like this, if you wish to directly assign the pointer:

  result = strtok(text,":");
  buff->name= strdup(result);

strdup is POSIX function. So if it's not available you can easily implement it.

edited Dec 14, 2012 at 13:22

answered Dec 14, 2012 at 13:16

P.P

122k20 gold badges193 silver badges251 bronze badges

1 Comment

Fred Foo Over a year ago

The memory allocation is fine (except for the lack of error checking). It's the copying of contents that is missing.

Alexander Atanasov · Accepted Answer · 2012-12-14 13:17:11Z

1

you need to copy the text in result in the newly allocated memory block. now you just allocate memory (assign the pointer to the pointer), then you discard that pointer and assign a new pointer result to it. you need to strncpy the result to the new memory allocation. Also strlen does not include the terminating zero byte, so you do not have enough space in the new memory block. There is a function strdup which does what you want.

answered Dec 14, 2012 at 13:17

Alexander Atanasov

4964 silver badges7 bronze badges

Collectives™ on Stack Overflow

C Allocating memory for array of structures containing multiple char*

5 Answers 5

1 Comment

Comments

2 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

Comments

2 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related