CODEWITHSUNDEEP
cmemorymemory-managementmallocvalgrind
Manal Suleiman Abd Alaziz
Manal Suleiman Abd Alaziz

Reputation: 37

valgrind error with strcpy: invalid read of size 1

I am getting the following error:

Invalid read of size 1 at 0x4008F4: isNameValid (elections.c:76)

and it seems the error is caused by the strcpy function. yet I don't know why is it wrong, since my code wouldn't run correctly without it. So how can I solve this?

a minimal example 

bool isNameValid(const char *tribe_name) {
    char *str = malloc(sizeof (strlen(tribe_name) + 1));
    if (!str) {
        return false;
    }
    if (!strcpy(str, tribe_name)) {
        free(str);
        return false;
    }
}

PS: it works fine when tribe_name has less than 7 characters, but when I enter more than 7, I get the error above.

Upvotes: 1

Views: 462

Answers (2)

Paul Floyd
Paul Floyd

Reputation: 6936

You can save all that bother of making sure that you get the length of the character array correct by simply using strdup. Don't complicate matters unnecessarily!

Your code could simply be

bool isNameValid(const char *tribe_name) {
    char *str = strdup(tribe_name);
    if (!str) {
        return false;
    }
}

Upvotes: 0

RobertS supports Monica Cellio
RobertS supports Monica Cellio

Reputation: 15062

char* str = malloc(sizeof (strlen(tribe_name) + 1));

sizeof (strlen(tribe_name) + 1) apparently evaluates to sizeof(size_t) in your case because sizeof yields the size in bytes of the object representation of the type of the expression and sizeof(size_t) is on modern 64-bit systems with 8 byte larger than sizeof(int) with 4 byte.

Since the return type of strlen is of type size_t and 1 is an int and per majority rule the int value will be promoted to size_t (the larger type) before the expression is evaluated, if sizeof(size_t) is larger than sizeof(int), the result of (strlen(tribe_name) + 1) will be of type size_t.

So memory of the size of a size_t object with 8 bytes is allocated with that call to malloc().

In 8 bytes you can only store a string of 7 characters plus the required string-terminating null character.

If you attempt to store a string of more than 7 characters in that memory, you will write beyond the bounds of the allocated memory, which is wrong.

Instead, It shall be 

char* str = malloc(sizeof(char) * (strlen(tribe_name) + 1));

Upvotes: 2

Related Questions