double pointer typedef struct array

Question

I am new to c programming and I am stuck with this one its a typedef struct and what I would like to do is that I want to create an array from the double pointer from this structure

typedef struct
{
 char* firstname;
 float   price;
}Name,*pName,**ppName;

typedef struct
{
 ppName Names;
 unsigned int numPerson;
}Book;

And my main which always give me segmentation fault dont mind the loop it is looping until the use says to quit.

 int main(void)
{

 Book D;
 setUpCollection(&D);

  while(..)
 {


  scanf(...);
  switch(...)
  {
  case 1:
   if(!AddNewPerson(&D))
    return 1;
   break;
  case 2:
  ....
  case 3:
   ....
  default:
   printf("Please enter a valid choice");
  }
 }
 return 0;
}

void setUpCollection(Book* data){
 Name name;
 pName pname;


 pname= malloc(MAX_PERSON* sizeof(pName));

 pname= &name;

 data->Names= &pname;
 data->numPerson= 0;

}

BOOL AddNewPerson(Book* data){
 char *title = malloc(sizeof(char));
 int len;
 Name name;
 pName pname;


 scanf(...);
 len = strlen(firstname);

 name.firstname = malloc(len * sizeof(char*));
 name.firstname  = firstname;

 pname= malloc(1);
 pname= &name;

 data->DVDs[data->numPerson++] = pname;

  printf("%0.2f", data->Names[(data->numPerson)-1]->price); 

 return TRUE;
}

My main problem is that I cant print all the added names and also getting segmentation fault.

sorry i was wirting the code not cpying and pasting it... fixed... — game on
– game on, Commented Jan 9, 2014 at 6:03
pname= malloc(1); ?? data->DVDs[data->numPerson++] = pname; ??? — Sourav Ghosh
– Sourav Ghosh, Commented Jan 9, 2014 at 6:06
You should go ahead and copy whatever code you've got. What you have up here now isn't helpful. — Keeler
– Keeler, Commented Jan 9, 2014 at 6:07
I will im really sorry i am really bad at pointers which i hate the most — game on
– game on, Commented Jan 9, 2014 at 6:11
Using typedefs to hide pointers makes matters worse, in my opinion. I would not use the pName or ppName types unless someone forced me to do so, and I'd write the code without them, and place them in when necessary before submitting the code for scrutiny. They are not IMNSHO a good idea. It also helps if you provide compilable code that reproduces your problem (see SSCCE – Short, Self-Contained, Correct Example). Comments about 'never mind the loop' mean 'worry about the loop' because, depressingly often, the trouble is in the bit that you're not supposed to mind. — Jonathan Leffler
– Jonathan Leffler, Commented Jan 9, 2014 at 6:59

AndersK · Accepted Answer · 2014-01-09 08:15:19Z

2

There are quite a few errors in your program but let me mention a few:

Doesn't this seem odd to you:

pname= malloc(MAX_PERSON* sizeof(pName));
pname= &name;

you are creating a memory leak by first letting pname point to the array of pName then assigning to &name.

What is this:

char *title = malloc(sizeof(char)); // ?

here you allocate too less space

name.firstname = malloc(len * sizeof(char*));

it should be

name.firstname = malloc(len * sizeof(char) + 1);

or more readable:

name.firstname = malloc(len+1);

this makes no sense again:

pname= malloc(1);
pname= &name;

again you created a memory leak by first letting pname point to a heap block of 1 byte then assigning it to a local variable which you include in data - the local variable is freed up once you leave AddNewPerson() so data will point to garbage.

Instead do something like this (I am no fan of having typedefs for pointers), also try avoiding naming types the same way you name variables for clarity:

typedef struct
{
  char *firstname;
  float price;
} Name;

typedef struct
{
  Name** names;
  unsigned int numPerson;
} Book;

Now allocate the initial size of your array, the whole point of having it on the heap is that the array can grow if more records are added than MAX_PERSONS so you need to keep track of the number of used records in the array as well as the number of records allocated

int allocated = MAX_PERSONS;
Book D;
D.names = malloc( allocated * sizeof(Name*) );
D.numPerson = 0;

then loop over user input and add records keeping track of how many records have been read. Since names is an array of pointers, you need to allocate a Name struct each time you add an entry

e.g.

D.names[i] = malloc( sizeof(Name) );
D.names[i]->firstname = strdup(userInputName);
D.names[i]->price = userInputPrice;

then at each iteration check if there is allocated memory left

++i;
if ( i == allocated )
{
  // if yes you need to get more memory, use realloc for that
  // get e.g. 10 more records
  Name* tmp = realloc( D.names, (allocated + 10)*sizeof(Name) ); 
  if ( tmp != NULL )
  {
    D.names = tmp;
    allocated += 10;
  }
  else 
  { .. some error msg .. }
}

edited Jan 9, 2014 at 8:15

answered Jan 9, 2014 at 6:21

AndersK

36.1k6 gold badges63 silver badges86 bronze badges

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

5 Comments

game on Over a year ago

what do you think the best approach just in the addnewperson function?. I am really bad at pointers. sorry

game on Over a year ago

sorry but i do really need the double pointer and pointer within the typededf thats the part i am stuck with.. BTW thx for the answer.

AndersK Over a year ago

ok if you replace the firstname[] with a pointer you just need to allocate that as well. you can e.g. use strdup to allocate a copy of the user input - updated answer

game on Over a year ago

are you able to do chat?? need to ask about it

AndersK Over a year ago

let us continue this discussion in chat

Collectives™ on Stack Overflow

double pointer typedef struct array

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related