2
\$\begingroup\$

Q: Please comment on the usage of structures and structure assignment operations in C

I am working on converting a MATLAB program to C using BLAS and LAPACK for linear algebra support. The MATLAB code uses cell arrays. I created a Matrix datatype and a Cell data-type.

A section of the header file/implementation:

#define ASSERT(c,m) 
#define PREC double

#define ZEROS(r,c) (zeros(r,c))
#define ONES(r,c) (ones(r,c))
#define EYE(r,c) (eye(r,c))
#define ALLOCM(r,c) (alloc_matrix(r,c))
#define PRINTM(M) (print_matrix(M))
#define FREEM(M) (free_matrix(M))

#define ALLOCC(r,c) (alloc_cell(r,c))
#define GETMC(C,r,c) (get_matrix_from_cell(C,r,c))
#define SETMC(C,r,c,M) (set_matrix_in_cell(C,r,c,M))
#define FREEC(C) (free_cell(C))

/* Matrix */
typedef struct {
  PREC * array;
  int rows; // The number of rows in the matrix
  int cols; // The number of columns in the matrix
}Matrix;


/* Cell of Matrices */
typedef struct {
  Matrix * array; // Cell array of matrices stored in row major form
  int rows;  // Number of rows in Cell array
  int cols;  // Number of cols in Cell array
}Cell;


/* Matrix utility functions */
Matrix  alloc_matrix(int rows, int cols);
Matrix  zeros(int rows, int cols);
Matrix  ones(int rows, int cols);
Matrix  eye(int rows, int cols);
Matrix  corrcov(Matrix matrix);
void print_matrix(Matrix matrix);
void free_matrix(Matrix  matrix);

/* Cell array utility functions */
Cell alloc_cell(int rows, int cols);
INLINE Matrix get_matrix_from_cell(Cell cell, int row, int col);
INLINE void set_matrix_in_cell(Cell  cell, int row, int col, Matrix matrix);
void free_cell(Cell cell); 

// Implementation
Matrix 
alloc_matrix(int rows, int cols){

  Matrix matrix;

  ASSERT(rows > 0 && cols > 0, FATAL_NEGATIVE_DIMENSIONS);

  matrix.array = (PREC *) malloc(sizeof(PREC) * rows * cols);

  ASSERT(matrix.array != NULL, FATAL_NO_MEMORY);

  matrix.rows = rows;

  matrix.cols = cols;

  return matrix;
}



Matrix 
zeros(int rows, int cols){

  int i;

  int size;

  Matrix matrix;

  matrix = alloc_matrix(rows, cols);

  for(i = 0, size = rows * cols; i < size ; i++){
    matrix.array[i] = 0.0;
  } 

  return matrix;
}



void
print_matrix(Matrix matrix){

  int i;  

  int j;

  int k;

  int rows = matrix.rows;

  int cols = matrix.cols;

  ASSERT(rows > 0 && cols > 0, FATAL_NEGATIVE_DIMENSIONS);  

  ASSERT(matrix.array != NULL, FATAL_NULL_POINTER);

  printf("\n Rows: %d, Cols: %d\n", rows, cols);

  for(i = 0 ; i < rows; i++){
    for(j = 0, k = i * cols; j < cols; j++){
      printf("%8.6f  ", matrix.array[ k + j ]);
    }
    printf("\n");
  }  

}


void 
free_matrix(Matrix matrix){

  ASSERT(matrix.array != NULL, FATAL_NULL_POINTER);

  free(matrix.array);

  matrix.array = NULL; 

}


Cell 
alloc_cell(int rows, int cols){

  Cell cell;

  ASSERT(rows > 0 && cols > 0, FATAL_NEGATIVE_DIMENSIONS);

  cell.array = (Matrix *) malloc(sizeof(Matrix) * rows * cols);

  ASSERT(cell.array != NULL, FATAL_NO_MEMORY);

  cell.rows = rows;

  cell.cols = cols;

  return cell; 
}



void 
free_cell(Cell cell){

  int i;

  int size;

  int rows = cell.rows;

  int cols = cell.cols;

  ASSERT(rows > 0 && cols > 0, FATAL_NEGATIVE_DIMENSIONS);

  ASSERT(cell.array != NULL, FATAL_NULL_POINTER);

  for( i = 0, size = rows * cols; i < size; i++){
    free_matrix(cell.array[i]);                       
  }

  free(cell.array);

}


INLINE Matrix 
get_matrix_from_cell(Cell cell, int row, int col){

  Matrix matrix;

  int rows = cell.rows;

  int cols = cell.cols;

  ASSERT(cell.array != NULL, FATAL_NULL_POINTER);

  ASSERT(rows > 0 && cols > 0, FATAL_NEGATIVE_DIMENSIONS);

  ASSERT(row >= 0 && row < rows, FATAL_INDEX_OUT_OF_BOUNDS);

  ASSERT(col >= 0 && col < cols, FATAL_INDEX_OUT_OF_BOUNDS);

  matrix = cell.array[(row * cols) + col];

  return matrix; 
}



INLINE void
set_matrix_in_cell(Cell cell, int row, int col, Matrix matrix){

  int rows = cell.rows;

  int cols = cell.cols;

  ASSERT(cell.array != NULL, FATAL_NULL_POINTER);

  ASSERT(rows > 0 && cols > 0, FATAL_NEGATIVE_DIMENSIONS);

  ASSERT(row >= 0 && row < rows, FATAL_INDEX_OUT_OF_BOUNDS);

  ASSERT(col >= 0 && col < cols, FATAL_INDEX_OUT_OF_BOUNDS);

  cell.array[(row * cols) + col] = matrix;

}

Notice that I do not return pointers from functions that allocate Matrix type objects or Cell type objects. I return a plain Matrix or Cell object, but I allocate the arrays (PREC type for matrices and Matrix type for cells - where PREC is double or float) inside these functions.

This is convenient because:

  1. struct assignment is a valid operation in C (I know that the dynamically allocated data is not duplicated, only a reference to it)

  2. to get to an element inside a Matrix, I can use the dot syntax instead of -> syntax (also I believe I may have to de-reference just once if I use the dot syntax - matrix.array[i] vs matrix->array[i] (I mean the matrix in matrix->array[i] is a pointer to a Matrix object, as I am comparing items on a stack vs those that are dynamically allocated)

  3. Most of the Matrix objects and Cell objects ( barring the dynamically allocated memory ) are on the stack and are automatically freed when function exits and I feel, its easier to keep track of objects that are no longer in use and free them when necessary.

I profiled a sample program (that uses this interface) with Valgrind.

The sample program:

#include "matutil.h"

int main(){

  Matrix mz;

  Cell cell;

  mz = ZEROS(3,3);

  PRINTM(mz);

  cell = ALLOCC(1,1);

  SETMC(cell,0,0,mz);  

  PRINTM(GETMC(cell, 0 , 0));

  FREEC(cell);

  return 0;
}

The problem is matrix object (Matrix mz) that I start with does not have its array field explicitly initialized and so, Valgrind reports

==17433== Conditional jump or move depends on uninitialised value(s)

Is this an issue? Are there any pitfalls that I should be aware of before I proceed with this design?

Thank you.

\$\endgroup\$
  • \$\begingroup\$ Okay. This is sorted now. The valgrind profile is hosted here: I get the Conditional Jump ... error when I do a plain alloc (not zeroing out the array) and no errors when the array is zeroes out. That probably explains why you don't get an error, because in the sample program I have given, it uses the zeros function to allocate memory, but I have been running my code with just alloc_matrix that just allocates memory but does not initialize it. filebox.vt.edu/users/maheshnm/MVN_RAND/valgrind-op-plainalloc filebox.vt.edu/users/maheshnm/MVN_RAND/valgrind-op-zeroalloc \$\endgroup\$ – mod0 Feb 27 '12 at 16:43
1
\$\begingroup\$

About your valgrind issue

I don't think the valgrind warning is about uninitialized fields in Matrix. Your zeros functions does the initialization, right? I tested myself with this code:

Matrix zeros(int rows, int cols) {
    Matrix tmp;
    int i;

    tmp.rows = rows;
    tmp.cols = cols;
    tmp.array = malloc(sizeof(float) * rows * cols);

    for(i = 0; i < rows * cols; i++)
        tmp.array[i] = 0;

    return tmp;
}

I have no valgrind warnings. Sharing a snippet of code wich really produces the valgrind warning would help.

Other remarks

  • If you want to free the memory automatically and have your allocated memory copied, C++ will help.
  • I don't understand your point about dot syntax though. You're only adding one level of nesting, this does not mean you're going to use "*" or "->" any less.
  • matrix->array[i] is wrong since -> and [] both dereference your pointer: this is not going to compile
  • Why don't you use a PREC** pointer? It is probably easier to use and will avoid errors
  • Are you trying to make your MATLAB code fast? Matrix operations are probably very fast using matlab. If there's another part of your code wich is slow, consider using MEX-files.
\$\endgroup\$
  • \$\begingroup\$ Hello, I have added the implementation to the code above. You are right about using * in place of ->. I can already see that I am using * more often. About matrix->array[i] - I wanted to compare a statically allocated matrix object and a dynamically allocated matrix object, I was not clear though. I meant to say matrix in matrix->array[i] was a pointer to a Matrix object. The easy solution is to implement Matrix ** inside a Cell object. And I am sure you must be right about MATLAB being faster. I may fall back on optimizing that code instead. :) \$\endgroup\$ – mod0 Feb 27 '12 at 13:58
  • \$\begingroup\$ The code that I am working on already uses a number of MEX components. I profiled it and found that the MEX components are the ones that are using the largest chunk of CPU. I just wanted to give ATLAS/LAPACK a shot. And besides the MATLAB code does numerous branching and loop stuff. If nothing works, I may have to MEX those portions. \$\endgroup\$ – mod0 Feb 27 '12 at 14:04
  • \$\begingroup\$ I still have no error (you forgot free_matrix and a few #defines like PREC or ASSERT, though). What line and file does Valgrind mention in your error? \$\endgroup\$ – Quentin Pradet Feb 27 '12 at 14:09
  • \$\begingroup\$ I am sorry, PREC and ASSERT were defined in a different file. I have disabled ASSERT for now, defined PREC as double. I have added free_matrix to the code. \$\endgroup\$ – mod0 Feb 27 '12 at 16:24
  • \$\begingroup\$ Okay. This is sorted now. The valgrind profile is hosted here: filebox.vt.edu/users/maheshnm/MVN_RAND/valgrind-op-plainalloc filebox.vt.edu/users/maheshnm/MVN_RAND/valgrind-op-zeroalloc I get the Conditional Jump ... error when I do a plain alloc (not zeroing out the array) and no errors when the array is zeroed out. That probably explains why you don't get an error, because in the sample program I have given, it uses the zeros function to allocate memory, but I have been running my code with just alloc_matrix that just allocates memory but does not initialize it. \$\endgroup\$ – mod0 Feb 27 '12 at 16:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.