CODEWITHSUNDEEP
cpointersstructsizeofdynamic-memory-allocation
lukassz
lukassz

Reputation: 3340

C iterate over struct bad data

I add values to my structure in while loop.

struct person
{
    char * name;
    char * surname;
    int  age;
    char * email;
};
struct person * tablicaOsob[100];

//for loop
tablicaOsob[i] = createPerson(name, surename, age, email);

Data has been added correctly. I check this via debugger

enter image description here

Now I would like iterate over tablicaOsob I create function

void list_persons(struct person *p, int k)
{
    printf("Lista osob\n");

    int i;
    for(i=0; i<k; i++)
    {
        printf("%s\n", p[i].name );
    }

}

But when I fire this function: list_persons(&tablicaOsob,i); I gets some bad data. Where is the problem. And my compilator says: 

main.c:74:17: warning: passing argument 1 of 'list_persons' from incompatible pointer type [enabled by default]
                 list_persons(&tablicaOsob,i);
                 ^
main.c:17:6: note: expected 'struct person *' but argument is of type 'struct person * (*)[100]'
 void list_persons(struct person *p, int k)

EDIT:

createPerson()

struct person * createPerson(char * name, char * surename, int age, char * email)
{

    struct person * p = (struct person *) malloc(sizeof(struct person *));

    p->name = name;
    p->surname = surename;
    p->email = email;
    p->age = age;

    return p;

}

Upvotes: 0

Views: 100

Answers (3)

Rishikesh Raje
Rishikesh Raje

Reputation: 8614

Your createPerson function is totally wrong.

  1. You are doing the initial malloc incorrectly, as mentioned by Alter Mann and Saurav.

  2. You need to also allocate memory for the pointers name, suname and email. This is required for allocation of multiple persons over a loop. You need to allocate memory of strlen(xxx) +1 for each character array.

The function will look like this:

struct person * createPerson(char * name, char * surename, int age, char * email)
{
  struct person * p = malloc(sizeof(struct person));

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

  p->surname = malloc(strlen(surename)+1);
  strcpy(p->surname, surename);

  p->email = malloc(strlen(email)+1);
  strcpy(p->email, email);

  p->age = age;

  return p;

}

Upvotes: 1

David Ranieri
David Ranieri

Reputation: 41046

struct person * p = (struct person *) malloc(sizeof(struct person *));

This is wrong, you want to reserve space for a struct, not for a pointer to this struct.

Change to

struct person *p = malloc(sizeof(struct person)); /* Don't cast malloc */

or

struct person *p = malloc(sizeof(*person));

This is also wrong:

list_persons(&tablicaOsob,i);

list_persons is expecting a pointer and tablicaOsob is an array of pointers, dont pass the address of an array of pointers to a single pointer, pass his value or simply pass the first element.

list_persons(*tablicaOsob,i);

Upvotes: 0

Sourav Ghosh
Sourav Ghosh

Reputation: 134376

Yes, the issue in in memory allocation. You're allocating way less memory to p than it needs. After that, accessing the returned pointer cause out of bounds memory access which in turn invokes undefined behavior.

In your code,

  struct person * p = (struct person *) malloc(sizeof(struct person *));

you are allocating memory only for the size of a "pointer to object", not for the size of "object" itself. You should write

struct person * p = malloc(sizeof(struct person));

or, for better

struct person * p =  malloc(sizeof*p);

Also, while calling the function, you should pass a pointer to the structure, something like

 list_persons(tablicaOsob[0],i); //tablicaOsob[0]is of type struct person *

should do the job, as long as i value does not cause out of bound access.

That said, Please see this discussion on why not to cast the return value of malloc() and family in C..

Upvotes: 2

Related Questions