-1

The following code crashes right before upon program exit. I have tested it on both MSVS 2015 and GCC. The program is just assigning a VLA on the heap (read about it here if you want) and reads a file contents character by character and storing this character in the array. The program works perfectly well. It does and prints everything correctly. However upon exiting it crashes, or it ceases to respond.

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

#define rows 8
#define columns 8

// allocate a VLA on the heap
void allocateVLArray(int x, int y, char(**ptr)[rows][columns])
{
    *ptr = malloc(sizeof(char[rows][columns]));
    assert(*ptr != NULL);
}

int main()
{
    char (*grid)[rows][columns];
    allocateVLArray(rows, columns, &grid);

    if (grid) {
        FILE *inputFile = fopen("test_fgetc.txt", "r");
        if (inputFile) {
            int x = 0, y = 0, length = 0;
            char ch;

            while((ch = (char)fgetc(inputFile)) != EOF) {
                // CR and LF characters are captured together (if necessary) and counted as one char using '\n'
                if (ch == '\n') {
                    x++; y = 0;
                }
                else {
                    *grid[x][y] = ch;
                    y++;
                }
                length++;
            }

            for (x = 0; x < rows; x++) {
                for (y = 0; y < columns; y++) {
                    printf("%c", *grid[x][y]);
                }
                printf("\n");
            }

            printf("\nlength = %d\n", length);
        }
    }

    free(grid);

    return 0;
}

I've also noticed my constant memory usage has increased significantly, which means memory leaks. So It is probably a heap problem. Why is this happening and how can i fix it?

trincot
  • 317,000
  • 35
  • 244
  • 286
KeyC0de
  • 4,728
  • 8
  • 44
  • 68
  • None of the code inside if (grid) {} should be allocating memory, unless there's something wrong with your library's printf() implementation. – A. L. Flanagan Mar 18 '17 at 01:07
  • 1
    `*grid[x][y]` --> `(*grid)[x][y]`. Also `*grid` isn't VLA. – BLUEPIXY Mar 18 '17 at 01:09
  • it's not crashing for me btw, but I don't have your test_fgetc.txt – transient_loop Mar 18 '17 at 01:12
  • your `allocateVLArray` accepts `int x, int y` and it does nothing with them. This means that you probably do not have all warnings enabled. (`-Wall` compiler parameter.) – Mike Nakis Mar 18 '17 at 01:15
  • The virtually universal conversion is that y counts rows and x counts columns. Please do not invent your own conventions. Or at least if you do, then do not take that to stackoverflow. – Mike Nakis Mar 18 '17 at 01:16
  • Also `char ch;` --> `int ch;` for comparison with `EOF`. – BLUEPIXY Mar 18 '17 at 01:17
  • Too complicated. Why not return the VLA? – too honest for this site Mar 18 '17 at 01:34
  • Show `test_fgetc.txt` – BLUEPIXY Mar 18 '17 at 08:49
  • @BLUEPIXY `grid` is a VLA allocated on the heap. Its a new thing (read that link i posted if you want). The mistake was the one you mentioned. I did `*grid[x][y]` and not `(*grid)[x][y]`. `[]` have higher precedence than `*`. I should have known. Everything else said here was wrong. `-Wall` was enabled, but `Wextra` was not. You can make it into an answer, so i can accept it. – KeyC0de Mar 18 '17 at 09:57
  • this line: `*ptr = malloc(sizeof(char[rows][columns]));` should be: `*ptr = malloc( rows*columns ); Note: it is not a VLA array if placed on the heap – user3629249 Mar 18 '17 at 15:39
  • the posted code fails to handle the case where the call to `fopen()` fails – user3629249 Mar 18 '17 at 15:44
  • this line: `while((ch = (char)fgetc(inputFile)) != EOF) {` will fail to catch `EOF if `char` is defined as `unsigned`. – user3629249 Mar 18 '17 at 15:45
  • for ease of readability and understanding: 1) follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* 2) separate code blocks (for, if, else, while, do...while, switch, case, default) via a single blank line. – user3629249 Mar 18 '17 at 15:49
  • what if the row in the file contains more than 8 characters? What if the file contains more than 8 rows? what if a row contains less than 8 characters? what if the file contains less than 8 rows? – user3629249 Mar 18 '17 at 15:49
  • when compiling, always enable all the warnings then fix those warnings. For instance, the function: `allocateVLArray()` has two unused parameters: `x` and `y`. – user3629249 Mar 18 '17 at 15:54
  • 1
    @RestlessC0bra `*grid` isn't VLA. Because `char (*grid)[rows][columns];` : `rows` and `columns` are defined by macro as constant `8`, not variable. So `char (*grid)[rows][columns];` is fix array that same as `char (*grid)[8][8];` (Also I think that MSVC2015 does not support VLA yet.) – BLUEPIXY Mar 18 '17 at 17:33
  • 1
    So `malloc(sizeof(char[rows][columns]))` is fix array allocated on heap, not _VLA allocated on the heap_. – BLUEPIXY Mar 18 '17 at 17:41
  • @BLUEPIXY Post an answer, so i can accept it. – KeyC0de Mar 18 '17 at 18:02
  • You can post your answers yourself. ;-) – BLUEPIXY Mar 18 '17 at 18:10

1 Answers1

0

What is probably happening is that your "test_fgetc.txt" contains more than 64 characters, or more than 8 rows of characters. That would exhibit precisely the behavior that you are experiencing: it would appear to work, and it would crash on free().

Mike Nakis
  • 56,297
  • 11
  • 110
  • 142