memory allocation eror in C

Question

I have been working on the following code for a while and there is a problem when I tried to allocate/deallocate memory for a struct and its elements. Any insights into the problem would be greatly appreciated thanks. Looking at the error message, I believe the problem lies in the fact that I tried to free an element that I had not properly allocated memory for but it is not obvious to me while looking at the code. I also tried code where I did not individually allocate memory for each element of the struct but that did not work as well.

typedef struct {
  char *cnet;
  char *email;
  char *fname;
  char *lname;
  char *tel;
} vcard;

vcard *vcard_new(char *cnet, char *email, char *fname, char *lname, char *tel)
{
  vcard* new = (vcard*)malloc(sizeof(vcard));
  printf("%lu\n", sizeof(new->tel) );
  new->cnet = malloc(sizeof(new->cnet));
  new->email = malloc(sizeof(new->email));
  new->fname = malloc(sizeof(new->fname));
  new->lname = malloc(sizeof(new->lname));
  new->tel = malloc(sizeof(new->tel));
  new->cnet = cnet;
  new->email = email;
  new->fname = fname;
  new->lname = lname;
  new->tel = tel;
  return new;
}

/* vcard_free : free vcard and the strings it points to
 */
void vcard_free(vcard *c)
{
  free(c->cnet);
  free(c->email);
  free(c->fname);
  free(c->lname);
  free(c->tel);
  free(c);
  return;
}

Answer 1

Your entire memory allocation is erroneous. Here are some pointers.

1. You allocate memory for only one char *, which is not what is intended.

   • Lesser memory allocation, possibility of boundary overrun.

2. Then, you overwrite the allocated memory by assigning the parameters to the same variables which held the pointer to allocated memories.

   • You end up causing memory leak.
3. You try to free the pointers which are not returned by malloc() and family.
   • Invalid attempt to free(), causes undefined behavior
4. You have used wrong format specifier in printf()
   • sizeof yields a result of type size_t, you must use %zu to print the result.

---

Solutions:

1. Allocate enough memory to store the expected content, like in predefined sizes

    #define CNETSIZ 32
    #define EMAILSIZ 64
    . . . . . 
    new->cnet = malloc(CNETSIZ);
    new->email = malloc(EMAILSIZ);
   

   or, based on the length of the input string, like

    new->cnet = malloc(strlen(cnet)+1);  //+1 for the space to null-terminator
   

2. Inside the vcard_new() function, use strcpy() to copy the content from the function parameters, like

   strcpy(new->cnet, cnet);
   strcpy(new->email, email);