-1
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>

typedef struct _person
{
    char *fname;
    char *lname;
    bool isavailable;
}Person;


Person *getPersonInstance(void)
{
    Person *newPerson = (Person*) malloc(sizeof(Person));
    if(newPerson == NULL)
        return NULL;
    return newPerson;
}

void initializePerson(Person *person, char *fname, char *lname, bool isavailable)
{
    person->fname = (char*) malloc(strlen(fname)+1);
  /*problematic behaviour if i write: person->fname = (char*) malloc (sizeof(strlen(fname)+1)); */

    person->lname = (char*) malloc(strlen(lname)+1);
 /*problematic behaviour if i write: person->lname = (char*) malloc (sizeof(strlen(lname)+1)); */

    strcpy(person->fname,fname);
    strcpy(person->lname,lname);
    person->isavailable = isavailable;

    return;

}

// test code sample
int main(void)
{
    Person *p1 =getPersonInstance();
    if(p1 != NULL)
        initializePerson(p1, "Bronze", "Medal", 1);

    Person *p2 =getPersonInstance();
    if(p2 != NULL)
        initializePerson(p2, "Silver", "Medalion", 1);

    Person *p3 =getPersonInstance();
    if(p3 != NULL)
        initializePerson(p3, "Golden", "Section", 1);

    printf("item1=> %10s, %10s, %4u\n",p1->fname, p1->lname, p1->isavailable);
    printf("item2=> %10s, %10s, %4u\n",p2->fname, p2->lname, p2->isavailable);
    printf("item3=> %10s, %10s, %4u\n",p3->fname, p3->lname, p3->isavailable);

    return 0;
}

Inside initializePerson() if i use:

person->fname = (char*) malloc (sizeof(strlen(fname)+1));
person->lname = (char*) malloc (sizeof(strlen(lname)+1));

When these two codelines are enabled instead of the ones i am using in the source code above, i might get a run time error when i test the code using CodeBlocks IDE. Very likely the console freezes up and stops working. If i test the code using ubuntu terminal it works any day without a problem regardless of the size of the input data.

Question: (Now, assume that we are using the 2 pieces of code from the previous paragraph) i know that sizeof counts bytes, and strlen counts the number of characters until it finds null... BUT sizeof and strlen when used together inside malloc() do they cause a conflict in the background? What seems to be the problem? why the code has such an erratic,unreliable behavior? why?

Cœur
  • 37,241
  • 25
  • 195
  • 267
Mynicks
  • 213
  • 1
  • 9
  • [Don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) – phuclv Jun 28 '17 at 09:38
  • Why would the *size* of the length be the same as the actual *value* of the length? – Andrew Henle Jun 28 '17 at 10:38
  • BTW, in a case such as yours when it is needed to allocate memory for the string and then copy from another string, it is easier to use `strdup` which combines three functions: `strlen`, `malloc` and `strcpy` – Eugene Jun 28 '17 at 11:12

2 Answers2

5

sizeof(strlen(fname)+1) doesn't make any sense. It gives the size of the result type of strlen, which is an integer of 4 bytes. So you end up allocating too little memory.

Use this:

person->fname = malloc(strlen(fname)+1);
Lundin
  • 195,001
  • 40
  • 254
  • 396
4

sizeof evaluates to the storage size of its argument (which might be an expression of a type, or just the type itself). So, look closely what the argument is here:

strlen(fname)+1

This is an expression of type size_t. sizeof will give you whatever amount of bytes is needed to store a size_t (probably either 4 or 8).

What you want is enough storage for your string, so the version with only strlen() is the correct one. In the other case, you reserve just 4 or 8 bytes and then write to memory locations you didn't allocate -> undefined behavior.


On a side note: casting void * is explicitly not needed in C and is considered bad practice by many (but not all) C coders. See this classic quesion.