Is this a valid usage of structure assignment in C?

Question

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.

• mod0: 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

Answer 1

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.

• mod0: 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. :)
• mod0: 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.
• Quentin Pradet: 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?
• mod0: 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.
• mod0: 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.