Search code examples
carraysmatrixbinaryindices

Elements of matrix not being stored correctly after reading from a text file


I am currently trying to read a matrix from a text file in c and I am trying to store the elements of the matrix in a 2D array in c.

For example in a text file called "Matrix.txt" I currently have the following 3X16 matrix:

0   0   1   0   0   0   0   1   0   1   0   0   1   0   0   0   
0   0   0   1   0   0   1   0   1   0   0   0   1   0   0   0   
0   1   0   0   1   0   1   0   0   1   0   0   0   0   0   0   

I want to store the same exact matrix in c. The program that I implemented is below:

#include <stdio.h>
#include <stdlib.h>
#define rows_Matrix 3
#define cols_Matrix 16

int main()
{   
    unsigned **Matrix = (unsigned **)malloc(sizeof(unsigned *)*rows_Matrix); //Rows

    for (int i = 0; i < rows_Matrix; i++) //Rows
    {
         Matrix[i] = (unsigned *)malloc(sizeof(unsigned) * cols_Matrix); //Columns 
    }

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

    for (int i = 0; i < rows_Matrix; i++)
    {
        for (int j = 0; j < cols_Matrix; j++)
        {
            //Read elements from a text file. 
            if (!fscanf(file, "%d", &Matrix[i][j]))
            {
                break;
            }

        }
    }

         fclose(file);

         //Print Matrix.
         for (int i = 0; i < rows_Matrix; i++) 
         {
            for (int j = 0; j < cols_Matrix; j++)
            {
                printf("%d\t", Matrix[i][j]);
            }
            printf("\n");
         }


    return 0;

}

When the matrix is being stored and printed its as follows:

 0       0       1       0       0       0       0       1       0       1     0       0       1       0       0

 0

 0       0       0       1       0       0       1       0       1       0      0       0       1       0       0

 0

 0       1       0       0       1       0       1       0       0      1      0       0       0       0       0

 0

It should be stored and printed in this way:

   0    0   1   0   0   0   0   1   0   1   0   0   1   0   0   0   
   0    0   0   1   0   0   1   0   1   0   0   0   1   0   0   0   
   0    1   0   0   1   0   1   0   0   1   0   0   0   0   0   0   

I would truly appreciate knowing why the elements are not being stored correctly in the Matrix and I confirmed this when I did matrix multiplication.


Solution

  • It appears your tab separators are causing your output to wrap at your terminal edge, putting the final value in each row on a line of its own. Other than your failure to free the memory you use, your code is working.

    That doesn't mean you are in the clear, or couldn't do a few things in a more convenient way -- that would making freeing your memory a lot easier -- requiring only a single call to free.

    Instead of declaring unsigned **Matrix (a pointer to pointer to type), declare matrix as a pointer to array of unsigned [cols_Matrix]. (essentially a pointer to an array of COLS unsigned values. Then you need only allocate 3 rows to allocate all storage for your matrix.

    (I avoid typing so the variables in the example below are shorter...)

    For instance you can declare matrix as

    #define ROWS  3
    #define COLS 16
    ...
        unsigned (*matrix)[COLS] = NULL,    /* pointer to array of COLS elem */
            row = 0,                        /* row counter */
            col = 0;                        /* column counter */
    

    Now a single allocation is all that is needed:

        matrix = malloc (ROWS * sizeof *matrix);    /* allocate matrix */
        if (!matrix) {                          /* validate allocation */
            perror ("malloc-matrix");
            return 1;
        }
    

    (There is no need to cast the return of malloc, it is unnecessary. See: Do I cast the result of malloc?)

    The remainder is similar to what you have done. There is nothing wrong with reading formatted integers values from a file with fscanf and %u (or any numeric conversion specifier) as the numeric conversion specifier will consume all leading whitespace (newlines included). You do however need to explicitly check your rows/columns during the read to protect your memory bounds and prevent writing outside you allocated block, e.g.

        /* read while row < ROWS & good value read */
        while (row < ROWS && fscanf (fp, "%u", &matrix[row][col]) == 1) {
            if (++col == COLS) {    /* increment col, test against COLS */
                row++;              /* on match - increment row */
                col = 0;            /* reset col */
            }
        }
    

    Now you must validate that all your data was correctly read. Checking the final values of row == ROWS provides confirmation, e.g.

        if (row != ROWS) {  /* validate all data read */
            fprintf (stderr, "error: failed to read all %dx%d matrix.\n",
                    ROWS, COLS);
            return 1;
        }
    

    Then it is a simple matter of outputting the data for confirmation and freeing the memory you have allocated, e.g.

        for (unsigned i = 0; i < ROWS; i++) {    /* output matrix */
            for (unsigned j = 0; j < COLS; j++)
                printf ("%4u", matrix[i][j]);
            putchar ('\n');
        }
    
        free (matrix);  /* don't forget to free mem (single free!) */
    

    Putting it altogether, you could do something like the following:

    #include <stdio.h>
    #include <stdlib.h>
    
    #define ROWS  3
    #define COLS 16
    
    int main (int argc, char **argv) {
    
        unsigned (*matrix)[COLS] = {NULL},  /* pointer to array of COLS elem */
            row = 0,                        /* row counter */
            col = 0;                        /* column counter */
        /* read file provided as 1st argument (default stdin if no argument) */
        FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;
    
        if (!fp) {  /* validate file open for reading */
            fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
            return 1;
        }
    
        matrix = malloc (ROWS * sizeof *matrix);    /* allocate matrix */
        if (!matrix) {                          /* validate allocation */
            perror ("malloc-matrix");
            return 1;
        }
    
        /* read while row < ROWS & good value read */
        while (row < ROWS && fscanf (fp, "%u", &matrix[row][col]) == 1) {
            if (++col == COLS) {    /* increment col, test against COLS */
                row++;              /* on match - increment row */
                col = 0;            /* reset col */
            }
        }
        if (fp != stdin) fclose (fp);   /* close file if not stdin */
    
        if (row != ROWS) {  /* validate all data read */
            fprintf (stderr, "error: failed to read all %dx%d matrix.\n",
                    ROWS, COLS);
            return 1;
        }
    
        for (unsigned i = 0; i < ROWS; i++) {    /* output matrix */
            for (unsigned j = 0; j < COLS; j++)
                printf ("%4u", matrix[i][j]);
            putchar ('\n');
        }
    
        free (matrix);  /* don't forget to free mem (single free!) */
    
        return 0;
    }
    

    Example Input File

    $ cat dat/3x16mat.txt
    0   0   1   0   0   0   0   1   0   1   0   0   1   0   0   0
    0   0   0   1   0   0   1   0   1   0   0   0   1   0   0   0
    0   1   0   0   1   0   1   0   0   1   0   0   0   0   0   0
    

    Example Use/Output

    $ ./bin/matrix_3x16 dat/3x16mat.txt
       0   0   1   0   0   0   0   1   0   1   0   0   1   0   0   0
       0   0   0   1   0   0   1   0   1   0   0   0   1   0   0   0
       0   1   0   0   1   0   1   0   0   1   0   0   0   0   0   0
    

    Memory Use/Error Check

    In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

    It is imperative that you use a memory error checking program to insure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

    For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

    $ valgrind ./bin/matrix_3x16 <dat/3x16mat.txt
    ==24839== Memcheck, a memory error detector
    ==24839== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
    ==24839== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
    ==24839== Command: ./bin/matrix_3x16
    ==24839==
       0   0   1   0   0   0   0   1   0   1   0   0   1   0   0   0
       0   0   0   1   0   0   1   0   1   0   0   0   1   0   0   0
       0   1   0   0   1   0   1   0   0   1   0   0   0   0   0   0
    ==24839==
    ==24839== HEAP SUMMARY:
    ==24839==     in use at exit: 0 bytes in 0 blocks
    ==24839==   total heap usage: 1 allocs, 1 frees, 192 bytes allocated
    ==24839==
    ==24839== All heap blocks were freed -- no leaks are possible
    ==24839==
    ==24839== For counts of detected and suppressed errors, rerun with: -v
    ==24839== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
    

    Always confirm that you have freed all memory you have allocated and that there are no memory errors.

    Look things over and let me know if you have further questions.