Did you guys see any mistake from this small code!

Write your question here.
For some purpose, i need to use malloc() function to allocate a 2D array.
After implementation I want to re-check this small code with you. If you
have any recommendation, please feel free to drop by some lines.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
    #include <stdlib.h>
    #include<iostream>

    template <class type1, class type2>
    int allocateData2DbyMalloc(type1*** data2D, type2 nRow, type2 nColumn) {
	int size = (char*) & (*data2D)[1] - (char*) & (*data2D)[0];
	*data2D = (type1 * *)malloc(size * nRow); // sizeof(type1*)

	if (*data2D)
	{
		size = (char*) & (**data2D)[1] - (char*) & (**data2D)[0];
		for (int i = 0; i < nRow; i++) {
			(*data2D)[i] = (type1*)malloc(size * nColumn); // 
    sizeof(type1)
		}

		return (**data2D != NULL) ? EXIT_SUCCESS : EXIT_FAILURE;
	}
	else
	{
		return EXIT_FAILURE;
	}
    }

    template <class type1, class type2>
    void destroyMatrix2DbyFree(type1*** data2D, type2 numberOfRow) {
	for (int row = 0; row < numberOfRow - 1; row++)
		free((*data2D)[row]);
	free(*data2D);
    }

    int main()
    {
	allocateData2DbyMalloc(&matrix2D, n, n);
	if (matrix2D == nullptr) {
		std::cout << "Error in 2D Array" << std::endl;
		exit(0);
	}
        return 0;
    }
Last edited on
"Re-check"? You did not even ask your compiler as "first check"!
It could have said:
 In function 'int allocateData2DbyMalloc(type1***, type2, type2)':
15:3: error: expected ';' before '}' token
 In function 'int main()':
34:26: error: 'matrix2D' was not declared in this scope
34:36: error: 'n' was not declared in this scope


Why have the horrid ** visible? Why not hide/encapsulate implementation into a "container" class?

Why is the type2 a class? It is perfectly valid to have template <class type1, int numberOfRow>
What's going on at line 6? You haven't allocated data2D yet, so why are you accessing data2D[0] and data2D[1]? or perhaps I misunderstand. It looks like the function to supposed to populate data2D with a malloced 2D array of type1 objects.

I understand that this may be an assignment, but using malloc for this is a really really REALLY bad idea. If type1 has a constructor then it won't be called when you allocate the items. Similarly, if it has a destructor, that won't be called when you free them. Finally, malloc doesn't initialize the space that it returns, so it might be full of random bits of data from previous allocations. At the very least, you should probably set it to zeros or use calloc() instead.

Looks like the same train-wreck posted here as well.
https://cboard.cprogramming.com/cplusplus-programming/178220-need-help.html

Didn't listen there, don't suppose much listening will be done here either.
Here is a version of your code that compiles and seems to work using calloc() and free().

But this code is like a textbook example of why we have the standard containers.
- Why should the client have to pass nRow to the destroyMatrix2DbyFree()?
- Why isn't this a class so the constructor and destructor deal with it?
- I didn't feel like writing the free() code if the allocation fails half way through the rows.
- type1 constructors and destructors aren't called.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
#include <stdlib.h>
#include<iostream>

// pass a reference to the data2D pointer instead of a pointer.
template <class type1, class type2>
int allocateData2DbyMalloc(type1** & data2D, type2 nRow, type2 nColumn) {

    // Allocate space for nRows pointers to type1 and zero-initialize it
    data2D = (type1 **) calloc(nRow, sizeof(type1*));
    if (data2D == NULL) {
	return EXIT_FAILURE;
    }

    // Now point each row to an array of nColumn type1 objects
    for (size_t i = 0; i< nColumn; ++i) {
	data2D[i] = (type1*)calloc(nColumn, sizeof(type1));
	if (data2D[i] == NULL) {
	    // You should free what you've allocated.
	    return EXIT_FAILURE;
	}
    }
    return EXIT_SUCCESS;
}

template <class type1, class type2>
void destroyMatrix2DbyFree(type1** data2D, type2 numberOfRow) {
    for (unsigned row = 0; row < numberOfRow; row++)
	free(data2D[row]);
    free(data2D);
}

int main()
{
    double **matrix2D;
    unsigned n=10, m=5;
    allocateData2DbyMalloc(matrix2D, n, m);
    if (matrix2D == nullptr) {
	std::cout << "Error in 2D Array" << std::endl;
	exit(0);
    }
    destroyMatrix2DbyFree(matrix2D, n);
    return 0;
}

Dear Salem,
I am not an expat. I am trying to collect the advise on my little implemented code from different expats. It's my freedom to collect the advise from different expat from different online platform, just to be precise on my little implementation. what do you mean by the sentence "Didn't listen there, don't suppose much listening will be done here either"?. Can you explain it please?.
So you want advice from several different websites, and yet you refuse to USE the advice you are given.

malloc is not a performance enhancer. In your case it actually makes things much worse.

Learn C++ tools for C++ code. STL containers instead of your bastardized C junk code.
Advice?

Throw that code away. All of it.
Do not assume.
Learn modern C++. Properly.
Learn what the "cuda" really requires. Do not trust hearsay.
Dear dhayden and Keskiverto,
Thanks for recommendation. I fixed it. My code is ready now to lunch to solve a matrix. Specially, I fixed some common mistake from your given advise. Thanks again.
Last edited on
shafiul0304034 wrote:
I believe, Malloc is a performance enhancer
That belief contradicts reality.

The reason is explained below:
https://stackoverflow.com/questions/3664272/is-stdvector-so-much-slower-than-plain-arrays
The problem with that code was explained in the answers to that question. Also, compilers, library, and the language itself improved in the 13 years that passed since VS2005. Runnig the code from that question today, with gcc, I get (live demo https://wandbox.org/permlink/KlShntZZHzBjq5iS )
1
2
UseArray completed in 1.408 seconds
UseVector completed in 1.407 seconds
Last edited on
I changed 'malloc' and 'free' for 'new' and 'delete' ... and it gave exactly the same timing, with vastly simplified syntax.

Whether you use dynamic arrays or vectors, however, I would suggest that if you wanted to pass information between CPUs (with MPI) or GPUs (with CUDA) you don't use multi-dimensional arrays. Flatten all your 2-dimensional (and, definitely, 3-dimensional) arrays so that you use a contiguous chunk of memory.
When did Paul Pedriana write that text? 1998?
Has he had lectures since, with same title, but with message that C++ is good for games?

If so, does that affect the credibility of the text?
The reason is explained below:
https://stackoverflow.com/questions/3664272/is-stdvector-so-much-slower-than-plain-arrays

A SO thread that is over 9 years old, and the answers to the question show the OP made serious errors in determining vectors are slower than arrays.

Clearly you did not read the entire thread, only the parts that confirmed your bias.

9 years and compilers have improved considerably in creating C++ code that is as speedy as C code.

Arguing you know more than people who code for a living is not a good look for wanting assistance.
ok Sir.
Topic archived. No new replies allowed.