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.