Getting error when trying release heap-allocated memory

Question

I am working on a program that reads text, line by line from input file. Once the line is read, the program reverses order of words in that string, prints it to the output file and starts reading next line. My program reads only specific number of characters from one line, meaning that if line contains more characters then that specific number, all of them have to skipped until next line is reached. My program seems to work fine.

One of the task requirements is to use dynamically allocated arrays. That is the part where my main problem lies. Once I try to free heap-allocated memory, the program fails with error message that says: HEAP CORRUPTION DETECTED. It must be that I messed up something while working with them. However, I am unable to find the real reason.

#include <stdio.h>
#include <stdlib.h>

#define BUFFER_SIZE 255

int readLine(FILE** stream, char** buffer, int* bufferSize);
void reverseString(char* buffer, char** reverse, int bufferSize, int lastLine);


int main(int argc, char** argv)
{
    char* buffer = NULL;
    char* reverse = NULL;
    int bufferSize = 0;
    int lastLine = 0;

    FILE* intputStream = fopen(argv[1], "r");
    FILE* outputStream = fopen(argv[2], "w");

    if (intputStream == NULL || outputStream == NULL)
    {
        printf("Input or output file cannot be opened\n");
        return 0;
    }

    while (!feof(intputStream))
    {
        lastLine = readLine(&intputStream, &buffer, &bufferSize);

        reverse = (char*)malloc(sizeof(char) * bufferSize);
        if (reverse != NULL)
        {
            reverseString(buffer, &reverse, bufferSize, lastLine);
            fputs(reverse, outputStream);
        }

    }

    fclose(intputStream);
    fclose(outputStream);

    free(buffer);
    free(reverse);

    return 0;
}

int readLine(FILE** stream, char** buffer, int* bufferSize)
{
    char tempBuffer[BUFFER_SIZE] = { 0 };
    int lastLine = 0;

    if (*stream != NULL)
    {
        fgets(tempBuffer, BUFFER_SIZE, *stream);
        char ignoredChar[100] = { 0 };

        *bufferSize = strlen(tempBuffer);
        // Ignoring in the same line left characters and checking if this is the last line
        if (tempBuffer[(*bufferSize) - 1] != '\n')
        {
            fgets(ignoredChar, 100, *stream);
            if (!feof(*stream))
                lastLine = 1;
        }
        // Allocating memory and copying line to dynamically-allocated array
        *buffer = (char*)malloc(sizeof(char) * (*bufferSize));
        if (*buffer != NULL)
        {
            memcpy(*buffer, tempBuffer, (*bufferSize));
            (*buffer)[(*bufferSize)] = '\0';
        }
    }
    // Return whether or not the last line is read
    return lastLine;
}

void reverseString(char* buffer, char** reverse, int bufferSize, int lastLine)
{
    int startingValue = (lastLine ? bufferSize - 1 : bufferSize - 2);
    int wordStart = startingValue, wordEnd = startingValue;
    int index = 0;

    while (wordStart > 0)
    {
        if (buffer[wordStart] == ' ')
        {
            int i = wordStart + 1;
            while (i <= wordEnd)
                (*reverse)[index++] = buffer[i++];
            (*reverse)[index++] = ' ';
            wordEnd = wordStart - 1;
        }
        wordStart--;
    }

    for (int i = 0; i <= wordEnd; i++)
    {
        (*reverse)[index] = buffer[i];
        index++;
    }

    if (!lastLine)
        (*reverse)[index++] = '\n';
    (*reverse)[index] = '\0';
}

Please read [Why is “while( !feof(file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) — Some programmer dude, Nov 23 '22 at 11:52
You also allocate memory in a loop, but only `free` after the loop. How many allocations will be made? How many `free` calls will be made? You need to match each and every allocation with a call to `free`, or else you will have a *memory leak*. — Some programmer dude, Nov 23 '22 at 11:57
There's also no need to pass a pointer to `reverse` to the `reverseString` function. — Some programmer dude, Nov 23 '22 at 12:00

Some programmer dude · Accepted Answer · 2022-11-23T12:00:57.177

One of the problems is in readLine where you allocate and copy your string like this (code shortened to the relevant parts):

*bufferSize = strlen(tempBuffer);
*buffer = (char*)malloc(sizeof(char) * (*bufferSize));
(*buffer)[(*bufferSize)] = '\0';

This will not allocate space for the null-terminator. And you will write the null-terminator out of bounds of the allocated memory. That leads to undefined behavior.

You need to allocate an extra byte for the null-terminator:

*buffer = malloc(*bufferSize + 1);  // +1 for null-terminator

_{[Note that I don't cast the result, and don't use sizeof(char) because it's specified to always be equal to 1.]}

Another problem is because you don't include the null-terminator in the bufferSize the allocation for reverse in main will be wrong as well:

reverse = (char*)malloc(sizeof(char) * bufferSize);

Which should of course be changed to:

reverse = malloc(bufferSize + 1);  // +1 for null-terminator

Getting error when trying release heap-allocated memory

1 Answers1