1

In my program, I enable the user to decide the dimensions of their matrix and then input each cell's value in the matrix. This matrix is then saved to a .txt file as binary data. Sometimes, with certain matrices of certain values and dimensions, the matrix is not read back properly with the correct values in the correct cells.

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

int numberOfMatrices = 0;
int orderedMatricesInfo[100][1][2];
int matrixRows = 0;
int matrixCols = 0;

int** initMatrix(int rows, int cols);
void saveMatrix(int** matrix, int rows, int cols);
void* allocateMatrix(int rows, int cols);
int** readMatrix(int rows, int cols, int matrix[matrixRows][matrixCols], int matrixNum);
void displayMatrices();
void displayMatrix(int rows, int cols, int** matrix);

int main()
{

    int n, m;
    int** matrixPointer;

    printf("Please enter the number of rows in your matrix: \n");
    scanf("%d", &n);

    printf("Please enter the number of columns in your matrix: \n");
    scanf("%d", &m);

    matrixPointer = initMatrix(n, m);

    printf("%d\n", matrixPointer[0][0]);

    displayMatrices();

    printf("SUCCESS");
}

int** initMatrix(int rows, int cols)
{
    int matrix[rows][cols];
    int **matrixPointer;

    matrixPointer = malloc(sizeof(int*)*cols);

    for (int i =0; i < cols; ++i)
    {
        matrixPointer[i] = malloc(sizeof(int*)*cols);
    }

    for (int i = 0; i < rows; ++i)
    {
        for (int j = 0; j < cols; ++j)
        {
            printf("\nPlease enter element for [%d][%d]: \n", i, j);
            scanf("%d", &matrixPointer[i][j]);
        }
    } 

    ++numberOfMatrices;

    orderedMatricesInfo[numberOfMatrices-1][0][0] = rows;
    orderedMatricesInfo[numberOfMatrices-1][0][1] = cols;

    allocateMatrix(rows, cols);

    saveMatrix(matrixPointer, rows, cols);
    return matrixPointer;
}

void saveMatrix(int** matrix, int rows, int cols)
{   
    char fullFileName[100] = "matrix";
    char fileNameExtension[100] = ".txt";
    char strNum[100];

    sprintf(strNum, "%d", numberOfMatrices);

    strcat(strNum, fileNameExtension);
    strcat(fullFileName, strNum);

    FILE *file = fopen(fullFileName, "ab");

    for(int i=0; i<rows; ++i)
    {
        fwrite(matrix[i], sizeof(int), rows, file);
    }
    fclose(file);
}

void* allocateMatrix(int rows, int cols)
{
    return malloc(sizeof(int[rows][cols]));
}

int** readMatrix(int rows, int cols, int matrix[matrixRows][matrixCols], int matrixNum) 
{
    int **matrixPointer;
    matrixPointer = malloc(sizeof(int*)*cols);

    for (int i =0; i < cols; ++i)
    {
        matrixPointer[i] = malloc(sizeof(int*)*cols);
    }

    char fullFileName[100] = "matrix";
    char fileNameExtension[100] = ".txt";
    char strNum[100];

    sprintf(strNum, "%d", matrixNum);

    strcat(strNum, fileNameExtension);
    strcat(fullFileName, strNum);

    FILE *file;
    file=fopen(fullFileName, "rb");
    fread(matrix, sizeof(int[rows][cols]), 1, file); // read 1 2D-array

    for (int i = 0; i < rows; ++i)
    {
        for (int j = 0; j < cols; ++j)
        {
            matrixPointer[i][j] = matrix[i][j];
            printf("%d", matrixPointer[i][j]);
        }
    }

    allocateMatrix(rows, cols);

    return matrixPointer;
}

void displayMatrices()
{
    for (int matrixNum = 1; matrixNum <= numberOfMatrices; ++matrixNum)
    {
        char fullFileName[100] = "matrix";
        char fileNameExtension[100] = ".txt";
    
        char strNum[100];
        sprintf(strNum, "%d", matrixNum);
        strcat(strNum, fileNameExtension);
        strcat(fullFileName, strNum);

        matrixRows = orderedMatricesInfo[matrixNum -1][0][0];
        matrixCols = orderedMatricesInfo[matrixNum -1][0][1];

        int matrix[matrixRows][matrixCols];
        int** matrixPointer;

        matrixPointer = readMatrix(matrixRows, matrixCols, matrix, matrixNum);

        printf("matrix %d", matrixNum);
        displayMatrix(matrixRows, matrixCols, matrixPointer);
    }
}

void displayMatrix(int rows, int cols, int** matrix)
{
    for (int i = 0; i < rows; ++i)
    {
        printf("\n");
        for (int j = 0; j < cols; ++j)
        {
            printf("%d ", matrix[i][j]);
        }
    }
    printf("\n");
}

In the main function, when the user creates the matrix, the matrix is saved in a newly created file named "matrix1.txt" as binary data. However, it is not stored properly I think as when read back, the matrix is not shown correctly with the correct values. For example, if I create a matrix:

[12, 19, 17, 15,
120, 566, 214, 153,
124, 1, 2, 3]

And then attempt to display the matrix then the matrix above is not shown, instead this is shown:

[12, 19, 17, 120,
566, 214, 124, 1,
2, 0, 101792956, 1]

Have I assigned memory incorrectly into this program? What would the problem be? As it does manage to get the first 3 elements of the top row retrieved and shown correctly. I am new to C so I cannot pinpoint the problem. When the matrix is smaller like a 2x2 matrix, then it works fine...

Thank you!

  • `matrixPointer = malloc(sizeof(int*)*cols);` You need `rows` not `cols` number of pointers. You also need `int` not `int*` in `matrixPointer[i] = malloc(sizeof(int*)*cols);` (for `cols` number of `int` not `cols` number of pointers) – David C. Rankin Nov 11 '20 at 01:55
  • I applied the changes however now I get a segmentation fault if I attempt to make a 3x4 matrix? – Siddharth Chaudhary Nov 11 '20 at 01:59
  • 1
    It will take me a minute to go through the rest, those were what immediately stood out. `orderedMatricesInfo[numberOfMatrices-1][0][0] = rows;` looks quite suspicious. Please provide [A Minimal, Complete, and Verifiable Example (MCVE)](http://stackoverflow.com/help/mcve). (something I can copy and compile without having to create function prototypes and add headers, etc..) – David C. Rankin Nov 11 '20 at 02:01
  • mhm howcome? I should've noted that that 3d array is responsible for storing each array's number of rows and columns. When the matrix is created, then the numberOfMatrices variable is incremented and then the number of rows is stored in [1][0][0] and the number of columns of the matrix is stored in [1][0][1]. – Siddharth Chaudhary Nov 11 '20 at 02:04
  • That's why I say give me a minute to go though the remainder of the code. You have to change your loop after changing to `rows`, you must loop `for (int i =0; i < rows; ++i)` – David C. Rankin Nov 11 '20 at 02:05
  • Is this within initMatrix? – Siddharth Chaudhary Nov 11 '20 at 02:08

1 Answers1

2

Your code is still missing information, but your initMatrix() function has a large number of small issues. You use cols where rows should be used to allocate pointers. You fail to validate any of the allocations or user-inputs. That is a recipe for Undefined Behavior. Additionally, you don't provide the code to determine what numberOfMatrices is, or how orderedMatricesInfo is declared. With those caveats noted, you can fix the immediate errors in initMatrix() with the following:

int **initMatrix(int rows, int cols)
{
    int matrix[rows][cols];
    int **matrixPointer;

    /* allocate rows number of pointers */
    matrixPointer = malloc (sizeof *matrixPointer * rows);
    if (!matrixPointer) {                           /* validate EVERY allocation */
        perror ("malloc-*matrixPointer");
        return NULL;
    }

    /* you must allocate rows number of cols * sizeof(int) blocks
     * assigning the starting address to each of your allocated pointers.
     * (always use the dereferenced pointer to set type-size)
     */
    for (int i = 0; i < rows; ++i)
    {   /* use calloc on rows to initialize all bytes zero */
        matrixPointer[i] = calloc (sizeof *matrixPointer[i], cols);
        if (!matrixPointer[i]) {                    /* validate EVERY allocation */
            perror ("calloc-matrixPointer[i]");
            while (i--)                             /* on error, free prior memory */
                free (matrixPointer[i]);
            free (matrixPointer);
            return NULL;
        }
    }

    for (int i = 0; i < rows; ++i)
    {
        for (int j = 0; j < cols; ++j)
        {
            while (1) { /* loop continually until valid integer provided */
                printf ("\nPlease enter element for [%d][%d]: \n", i, j);
                
                /* validate EVERY user-input */
                if (scanf("%d", &matrixPointer[i][j]) == 1)
                    break;
                    
                fputs ("  error: invalid integer input.\n", stderr);
            }
        }
    } 
    
    /* your code does not provide information on the following */
    ++numberOfMatrices;

    orderedMatricesInfo[numberOfMatrices-1][0][0] = rows;
    orderedMatricesInfo[numberOfMatrices-1][0][1] = cols;

    allocateMatrix(rows, cols);     /* why allocate -- that's what you just did?? */

    saveMatrix(matrixPointer, rows, cols);
    
    return matrixPointer;
}

(untested since your code is incomplete. The code is commented explaining the additions)

Note, your allocateMatrix(int rows, int cols) simply leaks memory. The return isn't assigned to any pointer so you have lost the beginning address to the allocated block with no way to free it.

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.

Let me know if you have questions and please provide A Minimal, Complete, and Verifiable Example (MCVE) for further assistance.

Further Update on Errors

The remainder of your code has the same rows and cols problem where you attempt to allocate cols number of pointers (instead of rows number of pointers). However, the problem leading to your SEGFAULT was due to numberOfMatrices and the file you attempt to open being off-by-one. This was resolved by assuring your loops are consistently indexing 0 < numberOfMatrices and accommodating the file number with:

    matrixPointer = readMatrix(matrixRows, matrixCols, matrix, matrixNum + 1);

Adding 1 to the matrixNum passed to readMatrix. That is a simple way to keep the numbering straight. You code writes and reads the file correctly, but note you are Appending to the file, so you will have to further work to access each of the matricies stored in your output file (you should write the number of rows and cols to the file before each matrix is written, so you can read the file back in, reading the 2 int values to determine the size of the matrix that follows -- that is left to you.

As it sits, your display function will read the file assuming the first integers in the file are for the current matrix -- which isn't the case. The integer values read to display are the elements for the first matrix that was written to the file, which will not be the same as the current matrix values unless you are writing and reading the first set of values. Otherwise, the current matrix values are contained in the file after the bytes for all previous matricies that were written to the file. A more useful file format would be:

no_matricies rows cols ...... rows cols ......

Where the first integer value written contains the number of matricies written to the file, followed by the row, col and .... data for each matrix which would allow all matricies to be accessed and allow the first integer value in the file to be updated without re-writing the entire file. (that too is left for you to consider)

Your cleaned-up code is as follows. Note: I changed your use of sprintf and eliminated the unnecessary concatenation. I also simplified copying from matrix to matrixPointer using memcpy():

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

int numberOfMatrices = 0;
int orderedMatricesInfo[100][1][2];
int matrixRows = 0;
int matrixCols = 0;

int** initMatrix(int rows, int cols);
void saveMatrix(int** matrix, int rows, int cols);
void* allocateMatrix(int rows, int cols);
int** readMatrix(int rows, int cols, int matrix[matrixRows][matrixCols], int matrixNum);
void displayMatrices();
void displayMatrix(int rows, int cols, int** matrix);

int main()
{

    int n, m;
    int** matrixPointer;

    printf("Please enter the number of rows in your matrix: \n");
    if (scanf("%d", &n) != 1) {
        fputs ("error: invalid integer input.\n", stderr);
        return 1;
    }

    printf("Please enter the number of columns in your matrix: \n");
    if (scanf("%d", &m) != 1) {
        fputs ("error: invalid integer input.\n", stderr);
        return 1;
    }

    matrixPointer = initMatrix(n, m);

    // printf("%d\n", matrixPointer[0][0]);

    displayMatrices();

    printf("SUCCESS");
}

int **initMatrix(int rows, int cols)
{
    int **matrixPointer;

    /* allocate rows number of pointers */
    matrixPointer = malloc (sizeof *matrixPointer * rows);
    if (!matrixPointer) {                           /* validate EVERY allocation */
        perror ("malloc-*matrixPointer");
        return NULL;
    }

    /* you must allocate rows number of cols * sizeof(int) blocks
     * assigning the starting address to each of your allocated pointers.
     * (always use the dereferenced pointer to set type-size)
     */
    for (int i = 0; i < rows; ++i)
    {   /* use calloc on rows to initialize all bytes zero */
        matrixPointer[i] = calloc (sizeof *matrixPointer[i], cols);
        if (!matrixPointer[i]) {                    /* validate EVERY allocation */
            perror ("calloc-matrixPointer[i]");
            while (i--)                             /* on error, free prior memory */
                free (matrixPointer[i]);
            free (matrixPointer);
            return NULL;
        }
    }

    for (int i = 0; i < rows; ++i)
    {
        for (int j = 0; j < cols; ++j)
        {
            while (1) { /* loop continually until valid integer provided */
                printf ("\nPlease enter element for [%d][%d]: \n", i, j);
                
                /* validate EVERY user-input */
                if (scanf("%d", &matrixPointer[i][j]) == 1)
                    break;
                    
                fputs ("  error: invalid integer input.\n", stderr);
            }
        }
    } 
    
    /* move numberOfMatrices++ below the assignment */
    orderedMatricesInfo[numberOfMatrices][0][0] = rows;
    orderedMatricesInfo[numberOfMatrices][0][1] = cols;

    numberOfMatrices++;
    
    // allocateMatrix(rows, cols);     /* why allocate -- that's what you just did?? */

    saveMatrix(matrixPointer, rows, cols);
    
    return matrixPointer;
}

void saveMatrix(int** matrix, int rows, int cols)
{   
    char fullFileName[256];

    sprintf (fullFileName, "matrix-%02d.txt", numberOfMatrices);

    FILE *file = fopen(fullFileName, "ab");
    if (!file) {    /* validate file open for writing */
        perror ("fopen-file");
        return;
    }
    
    for(int i=0; i<rows; ++i)
    {
        fwrite(matrix[i], sizeof *matrix[i], cols, file);
    }
    
    if (fclose(file) == EOF)        /* always validate close after write */
        perror ("fclose-file");
}

void *allocateMatrix(int rows, int cols)
{
    return malloc(sizeof(int[rows][cols]));
}

int **readMatrix(int rows, int cols, int matrix[matrixRows][matrixCols], int matrixNum) 
{
    int **matrixPointer;
    matrixPointer = malloc(sizeof *matrixPointer * rows);   /* rows not cols!! */
    if (!matrixPointer) {                           /* validate EVERY allocation */
        perror ("malloc-*matrixPointer");
        return NULL;
    }

    for (int i =0; i < rows; ++i)                           /* rows not cols!! */
    {
        matrixPointer[i] = calloc (sizeof *matrixPointer[i], cols);
        if (!matrixPointer[i]) {                    /* validate EVERY allocation */
            perror ("calloc-matrixPointer[i]");
            while (i--)                             /* on error, free prior memory */
                free (matrixPointer[i]);
            free (matrixPointer);
            return NULL;
        }
    }

    char fullFileName[256];

    sprintf (fullFileName, "matrix-%02d.txt", matrixNum);

    FILE *file = fopen(fullFileName, "rb");
    
    if (!file) {    /* validate file open for reading */
        perror ("fopen-file");
        for (int i = 0; i < rows; i++)      /* on error free memory */
            free (matrixPointer[i]);
        free (matrixPointer);
        return NULL;
    }
    /* validate EVERY input */
    if (fread (matrix, sizeof(int[rows][cols]), 1, file) < 1) { // read 1 2D-array
        for (int i = 0; i < rows; i++)      /* on error free memory */
            free (matrixPointer[i]);
        free (matrixPointer);
        return NULL;
    }
    
    for (int i = 0; i < rows; ++i)
    {
        memcpy (matrixPointer[i], matrix[i], sizeof *matrixPointer[i] * cols);
        /*
        for (int j = 0; j < cols; ++j)
        {
            matrixPointer[i][j] = matrix[i][j];
            printf("%d", matrixPointer[i][j]);
        }
        */
    }

    // allocateMatrix(rows, cols);  /* not needed */

    return matrixPointer;
}

void displayMatrices()
{
    for (int matrixNum = 0; matrixNum < numberOfMatrices; matrixNum++)
    {
        char fullFileName[256];
        sprintf (fullFileName, "matrix-%02d.txt", numberOfMatrices);
    
        matrixRows = orderedMatricesInfo[matrixNum][0][0];
        matrixCols = orderedMatricesInfo[matrixNum][0][1];

        int matrix[matrixRows][matrixCols];
        int **matrixPointer;

        matrixPointer = readMatrix(matrixRows, matrixCols, matrix, matrixNum + 1);

        displayMatrix(matrixRows, matrixCols, matrixPointer);
    }
}

void displayMatrix(int rows, int cols, int** matrix)
{
    for (int i = 0; i < rows; ++i)
    {
        for (int j = 0; j < cols; ++j)
        {
            printf (" %2d", matrix[i][j]);
        }
        putchar ('\n');
    }
    putchar ('\n');
}

(note: there are a number of additional comments left in the code above, rather then detailing each suggested change in paragraph form)

Example Use/Output

$ ./bin/write_matrix
Please enter the number of rows in your matrix:
2
Please enter the number of columns in your matrix:
2

Please enter element for [0][0]:
1

Please enter element for [0][1]:
2

Please enter element for [1][0]:
3

Please enter element for [1][1]:
4
  1  2
  3  4

SUCCESS

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

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • after copying this into my code, I still get a segmentation fault...? – Siddharth Chaudhary Nov 11 '20 at 02:33
  • The segmentation fault is related to other portions of your code. If you post the rest, so I can see how `numberOfMatrices` is declared and initialized (assume `int` and `0`) and then what `orderedMatricesInfo` is (assume a 3D array of some dimension). Then I can validate the rest of your code. – David C. Rankin Nov 11 '20 at 02:35
  • `fwrite(matrix[i], sizeof(int), rows, file);` in `saveMatrix()` has the same `rows` / `cols` problem. You write `cols` number of `int` per-row. – David C. Rankin Nov 11 '20 at 02:37
  • I see. I just edited the code so it has the numberOfMatrices variable and all – Siddharth Chaudhary Nov 11 '20 at 02:38
  • I have determined that the displayMatrices function is where the problem lies and what causes the segmentation error... however I don't understand why – Siddharth Chaudhary Nov 11 '20 at 02:42
  • Likely because arrays are ZERO based in C, so you want `for (int matrixNum = 0; matrixNum < numberOfMatrices; ++matrixNum)` as your loop limits. – David C. Rankin Nov 11 '20 at 02:44
  • unfortunately, no that still doesn't solve the problem. I kept matrixNum as 1 though to denote the first matrix made and so the first (1st) file. It shouldn't cause a problem to accessing elements though as (matrixNum-1) being the index avoids it – Siddharth Chaudhary Nov 11 '20 at 02:47
  • I have understood by removing parts of the code and re-running it that the readMatrix function may be the source of the problem however I cannot decipher why – Siddharth Chaudhary Nov 11 '20 at 02:48
  • You also have the same `rows` / `cols` mix-up in your `readMatrix()` function. I'll drop a full update in 5 min. – David C. Rankin Nov 11 '20 at 02:49
  • I copied your code and ran it. It worked for a 4x4 matrix. But when I ran it for a 4x5 matrix, it failed and didn't correctly show the inputted values – Siddharth Chaudhary Nov 11 '20 at 03:40
  • 1 2 3 4 5 6 7 8 9 10 11 12 1 2 3 4 5 6 7 8 – Siddharth Chaudhary Nov 11 '20 at 03:40
  • It showed that rather than {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 , 13, 14, 15, 16, 17, 18, 19, 20} – Siddharth Chaudhary Nov 11 '20 at 03:41
  • Read my extended comments about how your file format is currently appending to the same file. So if you have written a 4x4 and then write a 4x5, your code will attempt to read and display the values from first 4x4 matrix -- grabbing the additional integers as needed. This is the problem with opening the file in `"ab"` mode instead of `"wb"` mode so the file is overwritten each time. You can do it either way, but if you use `"ab"`, you must have a way to compute the offset from the beginning of the file where each new matrix is written. – David C. Rankin Nov 11 '20 at 03:45