CODEWITHSUNDEEP
cpointersdynamic-memory-allocation
physicsbot123
physicsbot123

Reputation: 71

memory allocation eror in C

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;
}

Upvotes: 5

Views: 149

Answers (3)

christian mini
christian mini

Reputation: 1702

Using new->cnet = malloc(sizeof(new->cnet)) you are allocating an amount of memory for cnet string equals to the pointer size and not to the actual string length.

Upvotes: 3

Sourav Ghosh
Sourav Ghosh

Reputation: 134386

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.
  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);

Upvotes: 6

Blaze
Blaze

Reputation: 16876

You're just allocating enough memory to hold another char* instead of the actual string. And then you overwrite the pointer with the pointer passed. Later you try to free that pointer, which means that it will try to free the memory of which you took a pointer and passed it to the function. Instead you should do this:

vcard *vcard_new(char *cnet, char *email, char *fname, char *lname, char *tel)
{
  vcard* new = (vcard*)malloc(sizeof(vcard));
  new->cnet = malloc(strlen(cnet)+1);
  strcpy(new->cnet, cnet);
  ...
}

Or, if strdup is available on your system:

vcard *vcard_new(char *cnet, char *email, char *fname, char *lname, char *tel)
{
  vcard* new = (vcard*)malloc(sizeof(vcard));
  new->cnet = strdup(cnet);
  ...
}

Upvotes: 4

Related Questions