CODEWITHSUNDEEP
cstringvalgrind
GSchimiti
GSchimiti

Reputation: 43

Valgrind warning write and read errors

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.

Upvotes: 0

Views: 156

Answers (2)

WhozCraig
WhozCraig

Reputation: 66224

Follow the path of one of the invocations in question.

  1. 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.

  2. qsortString(string1, tempString1); : Now we pass both the source string buffer and the newly allocated one-char-too-short buffer to this function.

  3. 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

Upvotes: 1

Jiminion
Jiminion

Reputation: 5168

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

Upvotes: 1

Related Questions