3

I'm getting a memory problem in c++ code, for sure I'm wrong but I don't get the problem. I've declare an array of three pointer to double and allocate memory.

double *myDoubles[3];

for(int i=0;i<3;i++) {
   myDoubles[i]= (double *) malloc(1000*sizeof(double));
}

myDoubles[2][999]=10.55;

Whats wrong?

EDIT

As @EdHeal comments and also @PhilippKiener I should not be using malloc at all. But it's difficult to forget malloc before 25 years with it. After all my problem was another bug in another place and actually I was writing out of limits :). My fault!

Mquinteiro
  • 1,034
  • 1
  • 11
  • 31
  • 1
    In C you do not cast `malloc` - it is bad - see http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Ed Heal Apr 02 '17 at 12:04
  • What is the nature of the memory problem which you are getting? – Jay Apr 02 '17 at 12:06
  • @Jay The problem is that i'm getting memory corruption in other variable when I operate with my double, but I does not look like I'm doing a index out off limits assignation like myDoubles[2][1000] or similar things. – Mquinteiro Apr 02 '17 at 12:09
  • @EdHeal It is c++ not c. sorry I edit the tags, but I have read de thread, and and include #include and the problem goes. In c++ I have to put the cast. but I don't understand why the program compiles but works wrong. – Mquinteiro Apr 02 '17 at 12:16
  • 2
    If C++ - Why are you using `malloc`? You should be using `new` or better still `std::vector` – Ed Heal Apr 02 '17 at 12:19
  • @EdHeal you are right, but I'm very old with old manners – Mquinteiro Apr 02 '17 at 12:21
  • myDoubles[2][999]=10.55; -- This statement is correct for the memory which you are allocing. Can you post the complete code to see if problem exists elsewhere? – Jay Apr 02 '17 at 12:25
  • Do you free the allocated arrays later? – Meccano Apr 02 '17 at 12:25

3 Answers3

2

You allocated 1000 bytes each time instead of 1000 doubles. what you should do is:

malloc(1000 * sizeof(double));

Your complete code should look like this:

double* myDoubles[3];
for(int i = 0; i < 3; i++) {
    myDoubles[i] = (double*) malloc(1000 * sizeof(double));
}
myDoubles[2][999] = 10.55;
Meccano
  • 537
  • 4
  • 8
1

First things first: don't cast the pointer malloc returns. void* to (in your case) double* is implicit (and expected), casting it explicitly is generally considered bad practice.

Now on to your problem; malloc allocates the given number of bytes and returns a pointer to those bytes. You allocate 1000 bytes, but what you want is to allocate enough bytes for 1000 doubles. The reason for your error is that a double is larger than a single byte - it's 8 bytes. You can get the size of a type with sizeof(type); to allocate enough memory for your 1000 doubles, you'd therefore have to change the line inside the loop to:

myDoubles[i] = malloc(1000 * sizeof(double));

To get to know the size of the type on your system, you can put

printf("Size of double: %d", sizeof(double));

In your code.

Edit:

Now, since you're working with C++, you should not be using malloc at all. C++ makes life easier and gives you new!

The allocating line should be

myDoubles[i] = new double[1000];

The nice thing abour new is that it does not need the size supplied... you give it a type and an amount; it computes the bytes needed for you. But beware, instead of free you should use

delete[] myDoubles[i];
Phil Kiener
  • 778
  • 1
  • 7
  • 15
  • Sorry again @PhilippKiener I did question bad. bad tag, and a mistake in malloc example, in the code it is ok. I have to cast because is C++ – Mquinteiro Apr 02 '17 at 12:19
1

As you are using C++, use std::vector - See here

So the code becomes

std::vector<std::vector<double>> myDoubles(3);
myDoubles[2].resize(1000);
myDoubles[2][999] = 10.55;

Perhaps you need to resize the other entries - I leave that exercise to you.

In addition you have no worries about memory leaks.

Ed Heal
  • 59,252
  • 17
  • 87
  • 127