C, I am having a memory issue I think

Viewed 114

I have the following struct

struct matrix_t {
    int m;
    int n;
    double * data;
} matrix_t;

Which gets initialized and returned in a method.

struct matrix_t * makematrix(char * NAME){
    ... 
    static struct matrix_t data;
    data.m = m;
    data.n = n;
    data.data = &matrix[0][0];
    return &data;

}

I call this method from main

int main() {

    ...
    struct matrix_t * makematrix(...);
    printf("%lf\n", matrix1->data[4]);
    printmatrix(matrix1); // this is discussed right below

The correct element is printed out with printf. I then try to pass it to another call

printmatrix(matrix1);

which tries to do the exact same print statement

void printmatrix(struct matrix_t * matrix){
    printf("data point %lf\n", (matrix->data[4]));

But here it prints a 0.000 which is wrong. Is my data array getting deallocated? I've been banging my head. Whats going on.

Reproducing example

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


struct matrix_t {
    int m;
    int n;
    double * data;
} matrix_t;


//void printmatrix(int m, int n, double *matrix){
void printmatrix(struct matrix_t * matrix){
    printf("MATRIX: \n");
    printf("M %d \n", matrix->m);
    printf("N %d \n", matrix->n);
    printf("data point %lf\n", (matrix->data[4]));
    for (int i = 0; i < matrix->m; i++){
        for (int j = 0; j < matrix->n; j++){
            //printf("%d\t", i* matrix->n + j);
            //printf("%lf\t", matrix->data[i* matrix->n + j]);
        }
        printf("\n");
    }

}

struct matrix_t * readfile(char * NAME){
    // Function for reading input files and returning data.

    FILE *fptr;
    char * line = NULL;
    size_t len = 0;
    ssize_t read;
    int linenum = 0;
    //array size
    int m;
    int n;

    if ((fptr = fopen(NAME,"r")) == NULL){
        printf("Error! opening file");
        exit(1);
    }


    while ((read = getline(&line, &len, fptr)) != -1) {
        if (linenum==0){
            sscanf(&line[0], "%d %d", &m, &n);
        }
        linenum++;
    }

    //allocate mxn matrix
    double matrix[m][n];

    fclose(fptr);
    fptr = fopen(NAME,"r");
    linenum=0;
    fscanf(fptr, "%d %d", &m,&n);

    for (int i = 0; i < m; i++) {
        for (int j = 0; j < n; j++) {
            fscanf(fptr, "%lf", &matrix[i][j]);
        }
    }

    static struct matrix_t data;
    data.m = m;
    data.n = n;
    data.data = &matrix[0][0];



    printf("testing \n");
    printf("%lf\n", data.data[4]);

    return &data;
}



int main()
{
    char filename1[] = "Amat.dat";
    char filename2[] = "Bmat.dat";
    struct matrix_t * matrix1;
    //struct matrix_t * matrix2;
    matrix1 = readfile(&filename1[0]);


    printf("%lf\n", matrix1->data[4]);

    printmatrix(matrix1);
    //matrix2 = readfile(&filename2[0]);
    //printmatrix(matrix1);
    //printf("test %lf", matrix1[0][0]);
    return 0;

}

prints:

testing 
4.000000
4.000000
MATRIX: 
M 4 
N 4 
data point 0.000000

where a file might look like

2 2 
1 2 
3 4
1 Answers

Since it is not really clear what exactly the question is I will assume that you are not understanding why your minimal example prints what it does.

Sadly you describe a non-reproducible problem: if I use your example and the given dataset (2 2 \n 1 2 \n 3 4) I get (unsurprisingly):

testing
0.000000
0.000000
MATRIX: 
M 2 
N 2 
data point 0.000000

When printing "testing" and a datapoint (data.data[4]) in line 76 you go out of bounds of your 'defined' array. If you change the 4 to something between 0-3 the data is output correctly.

Now, having changed that the testing does work, but the printmatrix() still show our data point as 0.000

testing 
4.000000
4.000000
MATRIX: 
M 2 
N 2 
data point 0.000000

An explanation for that was already given as a comment to OP here. A much better example to see this is to change the output in main to:

printf("%lf\n", matrix1->data[0]);                    
printf("%p\n", matrix1->data);                                   
printf("%lf\n", matrix1->data[0]);

This corresponds to:

1.000000
0x7fffcf8f6b40
0.000000

As stated in the comments, this is called undefined behavior in C. Stuff might or might not work, nobody knows.

So, how do we fix it? I did remove all irrelevant lines of code in the example and added a solution (along with some QoL additions). Of course it depends heavily on what you want to achieve. Instead of declaring your matrix static in the readfile function (which might lead to obfuscated code), you can dynamically allocate the memory. I think that my solution is not pretty, but it should show sufficiently how to allocate memory.

//man getline
#define _POSIX_C_SOURCE 200809L

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


struct matrix_t {
    int m;
    int n;
    double * data;
};


void printmatrix(struct matrix_t * matrix){
  for (int i = 0; i < matrix->m; i++) {
    for (int j = 0; j < matrix->n; j++) {
      printf("%lf\t", matrix->data[i* matrix->n + j]);
    }
    putchar('\n');
  }
}

/*
 *  we assume that the file NAME contains a matrix of the form
 *  m n
 *  {DATA}
 *
 *  where {DATA} is a N x M Matrix, delimited by spaces
 *
 *  we do not check if the file corresponds to this format! beware of dragons
 *
 *  onError: set matrix to NULL
 */
void readfile(struct matrix_t * matrix, char * NAME){

    FILE *fptr;

    if (!(fptr = fopen(NAME,"r"))){
      perror("error! opening file");
      matrix = NULL;
      return ;
    }

    int n,m;

    fscanf(fptr, "%d %d", &m, &n);

    int matrix_size = n * m;
    matrix->data = calloc(matrix_size, sizeof(double));

    for (int i = 0; i < matrix_size; i++) {
      fscanf(fptr, "%lf", &matrix->data[i]);
    }

    matrix->n = n;
    matrix->m = m;

    return;
}



int main() {

  struct matrix_t * matrix = malloc(sizeof(struct matrix_t *));
  if(!matrix)
    return EXIT_FAILURE;

  readfile(matrix, "Amat.dat");
  if(!matrix)
    return EXIT_FAILURE;

  printmatrix(matrix);

  puts("testing");
  printf("%lf\n", matrix->data[0]);                    
  printf("%p\n", matrix->data);                                   
  printf("%lf\n", matrix->data[0]);

  free(matrix->data);
  free(matrix);
  return EXIT_SUCCESS;
}

Now we get:

1.000000        2.000000
3.000000        4.000000
testing
1.000000
0x5591fadc34b0
1.000000

At last, some side notes about your code (unrelated to the initial question):

  • you are including iostream, which is a C++ library, so either this is about C++ or the library should not be included
  • Even if (or maybe specifically because) this is a minimal example, you should clearly comment on why things happen and what purpose they serve, so it is easier to understand by others (future-you included)
Related