Valgrind warning write and read errors

Question

I'm having write/read problems with valgrind and I don't understand why. The errors occurs on the same code block all time, changing only the memory address. The code block is:

void stringModifier(char *string) {
    char *sourceString = string;
    char *destinyString = sourceString;

    while(*string != '\0') {
            *string = tolower(*string);

            if(*string != ' ') { *destinyString++ = *string; }

            string++;
    }

    *destinyString = '\0';
}

int qsortComparison(const void *a, const void *b) {
    return (*(char *)a - *(char *)b);
}

void qsortString(char *string, char *tempString) {
    strcpy(tempString, string);
    stringModifier(tempString);
    qsort(tempString, strlen(tempString), sizeof(char), qsortComparison);
}

void outputReader(char *string1, char *string2) {
    char *tempString1 = (char *) malloc (strlen(string1) * sizeof(char));
    char *tempString2 = (char *) malloc (strlen(string2) * sizeof(char));

    qsortString(string1, tempString1);
    qsortString(string2, tempString2);

    if(!strcmp(tempString1, tempString2)) { printf("V\n", string1, string2); }
    else { printf("F\n"); }
}

Every time I use outputReader and call qsortString, valgrind warn a write error at strcpy and after that warn a read error at stringModifier, occurring on the same memory address.

sourceString is on stringModifier scope and take information for tempString parameter of qsortString. But I think that the really problem was on strcpy.. — GSchimiti, Aug 27 '13 at 12:52

score 1 · Answer 1 · answered Aug 27 '13 at 12:55

You need to alloc strlen+1 to account for null terminator of string.

void outputReader(char *string1, char *string2) {
    char *tempString1 = (char *) malloc ((strlen(string1)+1) * sizeof(char));
    char *tempString2 = (char *) malloc ((strlen(string2)+1) * sizeof(char));

score 1 · Accepted Answer · edited May 23 '17 at 11:57

Follow the path of one of the invocations in question.

char *tempString1 = (char *) malloc (strlen(string1) * sizeof(char)); : This allocates a string buffer that is the number of characters in string1 NOT including the 0-terminator, Space for the terminator is required if you want to store a complete copy of the string, but you don't allocate it.
qsortString(string1, tempString1); : Now we pass both the source string buffer and the newly allocated one-char-too-short buffer to this function.
In qsortString(), you then strcpy(tempString, string); : This will write one additional char (the terminator) to memory you don't own. This is therefore undefined behavior.

Valrind is right. You're writing (then reading) memory you don't own. The allocation should include space for a 0-terminator. such as: malloc ((strlen(string1)+1) * sizeof(char)); Another option, though not part of the C-standard, is to use strdup(), which would handle both allocation and copying for you correctly. How you choose to approach the solution I leave to you.

Side Note: Don't cast malloc() return values in C

I understand now what happened. I forgot the 0-terminator of the copy operation. Thanks! — GSchimiti, Aug 27 '13 at 13:12

Valgrind warning write and read errors

2 Answers2