Test code for custom malloc

Question

I've been working on a custom malloc where the test code is the following.

#include <stdlib.h>
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
#include "../tst.h"
#include "../brk.h"
#include <time.h>
/* returns an array of arrays of char*, all of which NULL */
char ***alloc_matrix(unsigned rows, unsigned columns) {
 char ***matrix = malloc(rows * sizeof(char **));
 unsigned row = 0;
 unsigned column = 0;
 if (!matrix) abort();
 for (row = 0; row < rows; row++) {
 matrix[row] = calloc(columns, sizeof(char *));
 if (!matrix[row]) abort();
 for (column = 0; column < columns; column++) {
 matrix[row][column] = NULL;
 }
 }
 return matrix;
}
/* deallocates an array of arrays of char*, calling free() on each */
void free_matrix(char ***matrix, unsigned rows, unsigned columns) {
 unsigned row = 0;
 unsigned column = 0;
 for (row = 0; row < rows; row++) {
 for (column = 0; column < columns; column++) {
 /* printf("column %d row %d\n", column, row);*/
 free(matrix[row][column]);
 }
 free(matrix[row]);
 }
 free(matrix);
}
int main(int agrc, char **argv) {
 int i;
 srand(time(NULL));
 int randomnumber;
 int size = 1024;
 void *p[size];
 for (i = 0; i < size; i++) {
 randomnumber = rand() % 10;
 p[i] = malloc(1024 * 1024 * randomnumber);
 }
 for (i = size-1; i >= 0; i--) {
 free(p[i]);
 }
 int x = 1024;
 char *** matrix = alloc_matrix(x, x);
 free_matrix(matrix, x, x);
 return (0);
}

The code compiles and runs cleanly.

developer@1604:~/proj/openmalloc/overhead$ gcc gigtest.c developer@1604:~/proj/openmalloc/overhead$ ./a.out

But how can I improve it? I would like to make it easier to test the thing (my next work should be to add multithreading to the malloc implementation).

Should I use a testing framework instead?

• greybeard: Which version/decade of C are you aiming for: shouldn't that be void*?
• vnp: I don't see how this code tests anything, and what ../tst.h and ../brk.h are for.
• chux: char ***alloc_matrix() does not return an array of arrays of char* It returns a pointer to pointer to pointer to char. The key issue is does the calling code of alloc_matrix() needs to swap rows, re-size allocations or are all the pointers mean to be constant until free_matrix()? If the latter, then code should use a single malloc()
• greybeard: Don't "pre-declare" for-loop iteration variables: If nothing else, the availability of their value after the loop invites errors.

Answer 1

This code is very easy to understand, which is great! I like that you used full words for the variable names rather than abbreviating everything to single character names.

Don't Abort

In alloc_matrix(), if there is a failure to allocate memory, it calls abort(). This is going to make it very difficult to test! If it attempts to allocate and fails, it aborts the test. Usually a test will return success or failure. This is an important distinction because you may write tests that should fail to test the failure paths of your code.

Proper Types

I would change the types of the argument to this:

char*** alloc_matrix(const size_t rows, const size_t columns);

I made them all const because the function doesn't change their values. I also made them size_t which is the appropriate type to use when you want to represent a size.

More Tests

You should add some more tests. As mentioned above, you need to test cases that should fail, such as allocating a block that's the maximum 64-bit value (for example malloc(ULLONG_MAX);).

You should also test things like alignment. Many platforms have strict byte alignment requirements. For example, pointers can usually not have an odd start address, and often must be 4-byte, 8-byte, or 16-byte aligned. You may need to check the specifics of the platform you're testing on.

• Roland Illig: You misunderstood the purpose of this program. The OP wrote an allocator (not shown here) and the code shown here is supposed to test that allocator.
• user1118321: Eh, maybe you're right. I removed the irrelevant stuff. Clearly from the comments to the question, I'm not the only one who was confused!
• Niklas Rosencrantz: @user1118321 Thanks for the review. I do have more tests. it's it the file RUN_TESTS in the repository. I inherited some of the code and tests and I write the code in the question in order to leran how to write tests and test that I actually could overrider ("shadow") the system's builtin malloc.
• S.S. Anne: There's no need to add const to the size_t types as they're passed by value anyway. If they were passed by reference, it would be a different story.
• user1118321: They're there as an annotation to the someone reading the code who might call it. Not strictly necessary, but a clue to how they're used in the function.