-1

looking for some advice on a problem I've been trying to solve for hours. The program reads from a text file and does some formatting based on commands given within the file. It seems to work for every file I've tried except 2, which are both fairly large. Here's the offending code:

    /* initalize memory for output */
    output.data = (char**)calloc(1,sizeof(char*));

    /* initialize size of output */
    output.size = 0;

    /* iterate through the input, line by line */
    int i;
    for (i = 0; i < num_lines; i++)
    {
    /* if it is not a newline and if formatting is on */
    if (fmt)
    {
        /* allocate memory for a buffer to hold the line to be formatted */
        char *line_buffer = (char*)calloc(strlen(lines[i]) + 1, sizeof(char));

        if (line_buffer == NULL)
        {
            fprintf(stderr, "ERROR: Memory Allocation Failed\n");
            exit(1);
        }

        /* copy the unformatted line into the buffer and tokenize by whitespace */
        strcpy(line_buffer, lines[i]);
        char* word = strtok(line_buffer, " \n");

        /* while there is a word */
        while (word)
        {
            /* if the next word will go over allocated width */
            if (current_pos + strlen(word) + 1 > width)
            {
                /* make ze newline, increase output size */
                strcat(output.data[output.size], "\n");
                output.size++;
  ------->>>>>  output.data = (char**)realloc(output.data, sizeof(char*) * (output.size + 1));

Using gdb I've figured out the error is on the line with the arrow pointing to it, only thing is I can't figure out why it occurs. It only happens when the text file that is being formatted is large (716 lines), and it seems to happen on the final iteration (num_lines = 716). Any thoughts would be hugely appreciated. Thanks!

EDIT: Sorry folks, should have mentioned that I'm pretty new to this! Fixed some of the errors.

user3717031
  • 123
  • 2
  • 8
  • 4
    `(output.size + )`? This shouldn't even compile… – mafso Jul 29 '14 at 23:54
  • 2
    [In C, don't cast the result of `malloc` (or `realloc`)](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Some programmer dude Jul 29 '14 at 23:58
  • 1
    Also note that [`strncpy`](http://en.cppreference.com/w/c/string/byte/strncpy) might not always add the string terminator. In your case it will because you add one extra byte to copy, which makes your call nothing different from the normal [`strcpy`](http://en.cppreference.com/w/c/string/byte/strcpy) function. Same with your use of `strncat`, why use it that way instead of just plain `strcat`? – Some programmer dude Jul 29 '14 at 23:58
  • 1
    Also, you should not reassign the result of `realloc` to the same variable you use in the call. What if the reallocation fails and returns `NULL`? Then you loose the original pointer and have a memory leak. – Some programmer dude Jul 30 '14 at 00:01
  • 3
    `strncpy(line_buffer, lines[i], strlen(lines[i]) + 1);` is a garbled way of writing `strcpy(line_buffer, lines[i]);` – M.M Jul 30 '14 at 00:01
  • 2
    `strncat(output.data[output.size], "\n", 2);` : output.data[index] doesn't have reserved memory for this. – BLUEPIXY Jul 30 '14 at 00:02
  • 3
    The _realloc: invalid next size_ usually indicates memory corruption of some sort as detailed in http://stackoverflow.com/questions/23680334/what-is-a-glibc-free-malloc-realloc-invalid-next-size-invalid-pointer-error-and. Chances are you have a buffer overflow in this or code not shown that is corrupting `output.data` which happens to eventually manifest itself in this error from the `realloc()` call. – uesp Jul 30 '14 at 00:35

1 Answers1

0

The most immediate problem is:

strncat(output.data[output.size], "\n", 2);

as pointed out by BLUEPIXY. Currently output.data[output.size] is a null pointer 1, so you cannot strncat to it.

To fix this you could allocate some space:

output.data[output.size] = malloc(2);
if ( NULL == output.data[output.size] )
     // error handling...

strcpy(output.data[output.size], "\n");

However there might be another solution that fits in better with the rest of your function, which you haven't shown. (Presumably you allocate space somewhere to store word).

It would be helpful to update your post and show the rest of the function. Also make sure you are posting the exact code, as (output.size + ) does not compile. I guess this is a typo you introduced when trying to put those big arrows on your line.

1 Actually it is all bits zero, which is a null pointer on common systems but not guaranteed to be so for all systems.

M.M
  • 138,810
  • 21
  • 208
  • 365
  • 1
    Using `strdup` rather than `malloc` or `calloc` followed by `strcpy` (or even worse, `strncpy` or `strncat`) would improve your code's readability and make it easier to avoid errors. – M.M Jul 30 '14 at 01:50