fgets not reading EOL?

Question

So bascially I am writing a text-based rpg in C and I wanted to create a map system. Basically the function im having trouble with is reading in "text map" from a file that looks like this:

----------\n
|c  x    [\n
|   x    |\n
]        |\n
----------\0

Its basically built using a 2d array. *EDIT I added in what the map looks like in the actual array. Is it because I dont like terminate each line

This is the function im having trouble with:

char** readMap(char* map_to_read,int h, int w){
    FILE* fp;
    int a = 0, b = 0;
    char map_return[h][w];
    char* c;

    fp = fopen(map_to_read, "r");

    for(a = 0; a < h; a++){
        for(b = 0; b < w; b++){
            c = (char*)malloc(sizeof(char) * w);
            map_return[a][b] = fgets(c, w, fp);
            printf("%s", c);
        }
        free(c);
    }

    fclose(fp);
    return map_return;
}

Everything reads in fine until the end, because fgets() is not reading EOL. This is what a printf looks like from the inside: https://i.sstatic.net/8q4CE.png

Can i get a second pair of eyes for this?

Answer 1

Analysis

What's the value in w? For your shown data, it should be at least 12 to get the newline too (10 characters, newline and null). You're going to have problems later because you can't (safely) return the local array map_return, but that's a separate bug. Also, you should be getting type mismatch warnings on the map_return[a][b] = fgets(c, w, fp); line because map_return[a][b] is a char and fgets() returns a char *. And you can't afford to free(c) if you're saving a pointer to it. There are so many problems here...

You responded:

Basically it's array[h][w], so w represents the number of elements in one line of the array.

Which got the further response:

So you need two separate chunks of memory. One is used to read the line and validate it. It can be simply char line[128];. You then use if (fgets(line, sizeof(line), fp) == 0) { ...process EOF/error...}. And assuming that passes, you validate the line and when it passes the validation, then you can arrange to copy up to w characters from the line into the map_return array. You have to decide whether you are playing with strings (terminated with a '\0') or not. You can make a case for either. You then have to deal with the problem of 'not returning a local variable'.

Synthesis

I suggest you change the interface to the function so that the caller allocates the memory for it.

This code compiles (but has not been run). It doesn't do much validation on the line that it reads; you can decide what more to do.

#include <stdio.h>
#include <stdbool.h>

extern bool readMap(char* map_to_read, int h, int w, char map[h][w]);

bool readMap(char* map_to_read, int h, int w, char map[h][w])
{
    FILE* fp;

    if ((fp = fopen(map_to_read, "r")) == 0)
        return false;

    for (int a = 0; a < h; a++)
    {
        char line[128];
        if (fgets(line, sizeof(line), fp) == 0)
        {
            fclose(fp);
            return false;
        }
        for (int b = 0; b < w; b++)
        {
            // Validation
            if (line[b] == '\n' || line[b] == '\0')
            {
                fclose(fp);
                return false;
            }
            map[a][b] = line[b];
            printf("%c", line[b]);
        }
        putchar('\n');
    }

    fclose(fp);
    return true;
}

This code assumes you are not storing null-terminated strings in the map array.

Example call:

int h = 5;
int w = 10;
char map[h][w];

if (mapRead("somefile", h, w, map))
    ...process initialized map...
else
    ...report failure...

The error reporting from the function is minimal; you can improve it as much as you think necessary.