0

I have this C program attempting a text line reading function that should be able to deal with lines of arbitrary length. It works by maintaining a buffer, whose size is doubled whenever there is need for more room.

The actual method is here:

/*******************************************************************************
* Attempts to expand the line buffer. If succeeded, TRUE is returned.          *
*******************************************************************************/
static char* try_expand(char* buffer, int* p_buffer_length)
{
    *p_buffer_length *= 2;

    puts("Before realloc");

    char* s = realloc(buffer, *p_buffer_length);

    puts("After realloc");

    if (s)
    {
        return s;
    }

    // Once here, realloc failed.
    char* s2 = malloc(*p_buffer_length);

    if (!s2)
    {
        return NULL;
    }

    strncpy(s2, buffer, *p_buffer_length / 2);
    free(buffer);
    return s2;
}

I am working on Mac OS X, and whenever the buffer expansion takes place, the program crashes and the system reports:

malloc: *** error for object 0x100105568: incorrect checksum for freed object - object was probably modified after being freed.

Everything else is here:

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

#define HELP_FLAG           "-h"
#define VERSION_FLAG        "-v"
#define FLAG_DESC           "%-5s"
#define INITIAL_BUFFER_SIZE 8
#define FALSE               0
#define TRUE                (~FALSE)

/*******************************************************************************
* This routine removes all leading and trailing whitespace from a string,      *
* doing that in-place. (Total of two passes.)                                  *
*******************************************************************************/
static char* trim_inplace(char* start)
{
    return start;
    /*
    for (char* end = &start[strlen(start) - 1];
         isspace(*end) && end >= start; --end)
    {
        *end = '\0';
    }

    while (isspace(*start))
    {
        ++start;
    }

    return start;*/
}
/*******************************************************************************
* Processes a single line and handles everything needed for dealing with lines *
* of arbitrary length.                                                         *
*******************************************************************************/
static int process_line(char** p_buffer, int* p_buffer_length, FILE* file)
{
    size_t current_index = 0;

    for (;;)
    {
        char* ret = fgets(*p_buffer + current_index, *p_buffer_length, file);

        if (!ret)
        {
            //puts("!ret is true.");
            return FALSE;
        }

        // Find out whether we have a newline character, which would imply that
        // we have an entire line read.
        for (size_t i = 0; i < *p_buffer_length; ++i)
        {
            if ((*p_buffer)[i] == '\n')
            {
                //(*p_buffer)[i + 1] = '\0';
                puts(trim_inplace(*p_buffer));
                return TRUE;
            }
        }

        // -1 for skipping the NULL-terminator.
        current_index += *p_buffer_length - 1;
        char* new_buffer;

        // Once here, the current line does not fit in 'p_buffer'. Expand the
        // array by doubling its capacity.
        if (!(new_buffer = try_expand(*p_buffer, p_buffer_length)))
        {
            perror("Could not expand the line buffer");
            free(*p_buffer);
            exit(EXIT_FAILURE);
        }
        else
        {
            *p_buffer = new_buffer;
        }
    }
}

/*******************************************************************************
* Processes a file.                                                            *
*******************************************************************************/
static void process_file(char** p_buffer, int* p_buffer_length, FILE* file)
{
    while (!feof(file))
    {
        process_line(p_buffer, p_buffer_length, file);
    }
}

/*******************************************************************************
* Prints the help message and exits.                                           *
*******************************************************************************/
static void print_help()
{
    printf("Usage: trim [" HELP_FLAG "] [" VERSION_FLAG "] "          \
           "[FILE1, [FILE2, [...]]]\n"                                \
           "    " FLAG_DESC " Print the help message and exit.\n"     \
           "    " FLAG_DESC " Print the version message and exit.\n"  \
           "    If no files specified, reads from standard input.\n",
           HELP_FLAG,
           VERSION_FLAG);
}

/*******************************************************************************
* Prints the version string.                                                   *
*******************************************************************************/
static void print_version()
{
    printf("trim 1.618\n" \
           "By Rodion \"rodde\" Efremov 08.04.2015 Helsinki\n");
}


/*******************************************************************************
* Prints the erroneous flag.                                                   *
*******************************************************************************/
static void print_bad_flag(const char* flag)
{
    printf("Unknown flag \"%s\"\n", flag);
}

/*******************************************************************************
* Checks the flags.                                                            *
*******************************************************************************/
static void check_flags(int argc, char** argv)
{
    for (size_t i = 1; i < argc; ++i)
    {
        if (strcmp(argv[i], HELP_FLAG) == 0)
        {
            print_help();
            exit(EXIT_SUCCESS);
        }
        else if (strcmp(argv[i], VERSION_FLAG) == 0)
        {
            print_version();
            exit(EXIT_SUCCESS);
        }
        else if (argv[i][0] == '-')
        {
            print_bad_flag(argv[i]);
            exit(EXIT_FAILURE);
        }
    }
}

/*******************************************************************************
* The entry point for a trivial line trimmer.                                  *
*******************************************************************************/
int main(int argc, char** argv)
{
    check_flags(argc, argv);

    int buffer_length = INITIAL_BUFFER_SIZE;
    char* buffer = malloc(buffer_length);

    if (argc < 2)
    {
        // If realloc changes the location of memory, we need to know this.
        process_file(&buffer, &buffer_length, stdin);
        fclose(stdin);
        return EXIT_SUCCESS;
    }

    for (size_t i = 1; i < argc; ++i)
    {
        FILE* file = fopen(argv[i], "r");

        if (!file)
        {
            perror("Error opening a file");
            return (EXIT_FAILURE);
        }

        process_file(&buffer, &buffer_length, file);
        fclose(file);
    }
}

The only observation I have made, is that if the input line requires only one expansion of the line buffer, everything is O.K. However, if the input line is large enough to require at least two expansions, the program crashes. What am I doing wrong here?

coderodde
  • 1,269
  • 4
  • 17
  • 34
  • Just out of morbid curiosity, why do you think `malloc/free` will work when `realloc` won't? – paxdiablo Apr 09 '15 at 07:34
  • This kind of error is reported when heap corruption is discovered. In this case it is the debug version of `malloc` where the discovery is made. Note that `realloc` most likely will call `malloc` so this may very well happen from a branch of your code that calls `realloc`. But the heap corruption happens before being discovered and as the error message suggest this often happens if a pointer is written to after the memory pointed to by the pointer has been freed. – Martin Liversage Apr 09 '15 at 07:47
  • 1
    When you free memory with `free(pointer);` next step should be `buffer = NULL;` this simple rule helps you prevent usage of deallocated memory – VolAnd Apr 09 '15 at 08:30

1 Answers1

1

When you read further chunks in process_line(), you pass the wrong size:

fgets(*p_buffer + current_index, *p_buffer_length, file);

Should be

fgets(*p_buffer + current_index, *p_buffer_length - current_index, file);
chqrlie
  • 131,814
  • 10
  • 121
  • 189