Search code examples
cstringfilematrixdynamic-memory-allocation

wrong output while reading file


I'm trying to read the contents of a file into a matrix. As i will have several files with unknown amounts of rows and columns, i allocated memory for the matrix dynamically.

My code until now.

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

int main(void)
{
    char** map, chr;
    int column = 0, row = 0, columns = 0, rows = 0, total_elements = 0;

    FILE* file = fopen("file.txt", "r+");

    // count numbers of rows and columns
    while (chr != EOF)
    {   
        if (chr == '\n')
        {
            rows = rows + 1;
        }
        chr = getc(file);
        total_elements+=1;
    }

    rows += 1;
    
    //Dividing the total number of elements by the number of rows to find the number of columns
    columns = (total_elements/rows) - 1;

    // alocate space for matrix
    map = (char **) malloc(rows * sizeof(char *));

    // allocating space for each column of each row
    for (row = 0; row < rows; row++) {
        map[row] = (char *) malloc(columns * sizeof(char));
    }

    // file reading
    for (row = 0; row < rows; row++) {
        for (column = 0; column < columns; column++) {
            if (!fscanf(file, "%c", &map[row][column]))
                break;
            printf("%c", map[row][column]);
        }
        printf("\n");
    }

    fclose(file);
    free(map);


    return 0;

This is the file:

....*.....................
..........................
........*........*........
.....*....................
...............*....*.....
..*.......*...............
............*.............
..........................
..............*...........
..................*.......
..*.......*......*........
....*..*..................
...**.....................
..........*...............
....................*.....
..........................
....**....................
......................*...

­The expected output would be the contents of the file, but i get the output in the wrong format.

?Å É/Å @Q▓v            @Q
­?Å É/Å             xA¢v
­?Å É/Å  ÊÐ C Å └ Å µ
­?Å É/Å     
­?Å É/Å
­?Å É/Å         T(Å     P(
­?Å É/Å @Q▓v            @Q
­?Å É/Å             xA¢vx(
­?Å É/Å ╩  ╩ÈÐ  └ Å É&Å ug
­?Å É/Å ┼  ┼═Ð  └ Å É&Å ├
­?Å É/Å     
­?Å É/Å  H
­?Å É/Å 
­?Å É/Å 
­?Å É/Å 
░Å É/Å
ä
 H

I really don't know where i could have made the mistake to make this happen.


Solution

  • As for the type of file you are reading and the way you are allocating memory, there is no need to read the whole file twice:

    .....*....................
    ...............*....*.....
    ..*.......*...............
    ............*.............
    

    The data is line oriented and --- I suppose --- all lines have the same number of columns. And the data will be stored at

        char **map;
    

    an array of pointers, so each map[i] is char* and can hold a line of data.

    fscanf() was written for consume tabular data, with possible delimiters and different field types, like csv files with many int and float data separated by # for example. There is not much sense in using fscanf() to read chars and you could use fgetc() or fread() instead.

    about your code

    • as already told, you need to rewind the file to read the data again
    • also, as told, you need to test for '\n' since it is not data
    • always test fscanf() return against the number of specifiers

    The loop below would consume your file considering what I said above

        rewind(file);
        row = 0;
        column = 0;
        while (1 == fscanf(file, "%c", &map[row][column]))
        {
            if (map[row][column] == '\n')
            {   row += 1, column = 0;
                printf("\n");
            }
            else
            {   printf("%c", map[row][column]);
                column += 1;
            }
        };  // while();
        fclose(file);
    

    In general is more manageable to use the map data in a flat memory area, using just char*, in the C way, and storing the map line after line, instead of using char**

    a more flexible way

    Using the char** way, you can read the data in lines and keep the map as strings as it may be useful to display and use, while still using

    map[][]
    

    for reference.

    I will show you an example

    Also, since you said you have many file sizes you should consider passing the file name as a parameter

    To have more control you could use some encapsulation and put the data in a struct like

    typedef struct
    {
        int    cols;
        int    rows;
        int    size;
        char** map;
    
    } Grid;
    

    This way you can have a function like

    int   show_grid(Grid*,const char*);
    

    and write

        char title[100];
        sprintf(title, "\n    Map for %s\n",file_name);
        show_grid(&gr, title);
    

    To see on screen

    
        Map for file.txt
    
    [18 rows, 26 columns]
    ....*.....................
    ..........................
    ........*........*........
    .....*....................
    ...............*....*.....
    ..*.......*...............
    ............*.............
    ..........................
    ..............*...........
    ..................*.......
    ..*.......*......*........
    ....*..*..................
    ...**.....................
    ..........*...............
    ....................*.....
    ..........................
    ....**....................
    ......................*...
    

    And the code can be simple as 4 lines:

    void show_grid(Grid* g, const char* msg)
    {
        if (msg != NULL) printf("%s\n", msg);
        printf("[%d rows, %d columns]\n",
            g->rows, g->cols);
        for (int i = 0; i < g->rows; i+=1)
            printf("%s\n", g->map[i]);
        printf("\n");
    }
    

    determining the column size

        int ch = 0;
        for (gr.cols = 0; ch != '\n'; gr.cols += 1)
        {
            ch = fgetc(F);
            if (feof(F)) return -2;
        }
    

    You just need to find the end of the first line, since all rows have the same size and the memory is allocated in lines.

    In the case of using char* and a flat area it may the simpler just to allocate an area as large as the file, using stat or ftell to get the file size instead of reading the file twice.

    allocating memory in blocks of rows

    It is faster to use this way, and read the data using fgets() to consume a whole line per call. Also, since the data is not flat anyway, you can keep the lines as strings to simplify the display. see:

        // blocksize in lines
        int row = 0;
        while (!feof(F))
        {
            gr.map[row] = (char*)malloc(gr.cols);  // new row
            fgets(gr.map[row], gr.cols, F);
            gr.map[row][gr.cols - 2] = 0;
            row += 1;
            if (row == gr.size)
            {  // expand block
                int    new_size = gr.size + BLKSIZE;
                char** temp = (char**)realloc(gr.map, sizeof(char*)*new_size);
                if (temp == NULL) break;
                gr.map  = temp;
                gr.size = new_size;
            };
        };
        fclose(F);
    

    You can have a suitable BLKSIZE to not resize many times.

    passing the file name on the command line

    This way you can have a default, but also pass the file name in the command line so it can be used in a script. And since the display function conveniently shows the file name you can check any number of files easily as in

        // open file as 'F'
        const char* default_file_name = "file.txt";
        char        file_name[80];
        FILE*       F            = NULL;
        if (argc > 1) 
            strcpy(file_name, argv[1]);
        else
            strcpy(file_name, default_file_name);
    
        F = fopen(file_name, "r");
        if (F == NULL)
        {
            perror("Could not open file");
            return -1;
        }
    

    Example

    SO> .\f0-0922
    
        Map for file.txt
    
    [18 rows, 26 columns]
    ....*.....................
    ..........................
    ........*........*........
    .....*....................
    ...............*....*.....
    ..*.......*...............
    ............*.............
    ..........................
    ..............*...........
    ..................*.......
    ..*.......*......*........
    ....*..*..................
    ...**.....................
    ..........*...............
    ....................*.....
    ..........................
    ....**....................
    ......................*...
    
    SO> .\f0-0922 other.txt
    
        Map for other.txt
    
    [5 rows, 5 columns]
    ....*
    ...*.
    ..*..
    .*...
    *....
    

    code for the examle

    #define BLKSIZE 20
    
    #include <errno.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    typedef struct
    {
        int    cols;
        int    rows;
        int    size;
        char** map;
    
    } Grid;
    
    void  show_grid(Grid*,const char*);
    void  free_grid(Grid*);
    
    int main(int argc, char** argv)
    {
        // open file as 'F'
        const char* default_file_name = "file.txt";
        char        file_name[80];
        FILE*       F            = NULL;
        if (argc > 1) 
            strcpy(file_name, argv[1]);
        else
            strcpy(file_name, default_file_name);
    
        F = fopen(file_name, "r");
        if (F == NULL)
        {
            perror("Could not open file");
            return -1;
        }
    
        // define grid
        Grid gr = {0, 0, 0, NULL};
    
        // set 'cols' to column size
        int ch = 0;
        for (gr.cols = 0; ch != '\n'; gr.cols += 1)
        {
            ch = fgetc(F);
            if (feof(F)) return -2;
        }
        gr.cols = gr.cols + 1;  // add space to a terminating 0
        rewind(F);              // roll back 1st line
        gr.map = (char**)malloc((BLKSIZE * gr.cols) * sizeof(char*));  // initial size
        gr.size = BLKSIZE;
    
        // blocksize in lines
        int row = 0;
        while (!feof(F))
        {
            gr.map[row] = (char*)malloc(gr.cols);  // new row
            fgets(gr.map[row], gr.cols, F);
            gr.map[row][gr.cols - 2] = 0;
            row += 1;
            if (row == gr.size)
            {  // expand block
                int    new_size = gr.size + BLKSIZE;
                char** temp = (char**)realloc(gr.map, sizeof(char*)*new_size);
                if (temp == NULL) break;
                gr.map  = temp;
                gr.size = new_size;
            };
        };
        fclose(F);
    
        gr.rows = row;
        gr.cols -= 2;
    
        char title[100];
        sprintf(title, "\n    Map for %s\n",file_name);
        show_grid(&gr, title);
        free_grid(&gr);
    
        return 0;
    }
    
    void show_grid(Grid* g, const char* msg)
    {
        if (msg != NULL) printf("%s\n", msg);
        printf("[%d rows, %d columns]\n", g->rows, g->cols);
        for (int i = 0; i < g->rows; i += 1) printf("%s\n", g->map[i]);
        printf("\n");
    }
    
    void free_grid(Grid* g)
    {
        for (int i = 0; i < g->rows; i += 1) free(g->map[i]);
        free(g->map);
        g->map = NULL;
        return;
    }