0

I am dealing with a problem, that has to do with dynamic allocation, in c++. I have the following function to calculate the rms of a signal

void FindRMS(int points, double* signal_x, double* signal_y, int presamples, double* mean, double* rms)
{       
    //double fraction_presamples_RMS = 0.9; // fraction of presamples to calculate RMS
  //Safety condition
  if (presamples>points) {
    printf("\nERROR: Too many presamples!\n\n");
    return;
  }

  //Main procedure
  (*rms)  =0.;  
  (*mean) =0.;

  for (int i=0; i<presamples; i++) {
    (*mean)+=signal_y[i];
    (*rms)+=pow(signal_y[i],2);
  }

  (*mean)/=presamples;
  (*rms)=sqrt((*rms)/presamples-pow((*mean),2));
    cout << "RMS was found to be : " << (*rms) << endl;

}

First of all, if I understand correctly, double* <var> means that the argument is expected to be dynamically defined, which means that its size will be limited by the hardware.

Then what I do, is to call this function in my code. A sample code is the following

void Analyze(unsigned int first_run, unsigned int last_run, unsigned int last-segment){

    int points = 9e6;//hSignal->GetNbinsX();
//double x[points], y[points], derivative[points]; // SIZE limited by COMPILER to the size of the stack frame
    double* x          = new double[points];           // SIZE limited only by OS/Hardware
    double* y          = new double[points];
    double* derivative = new double[points];
    double* mean       = new double[points];
    double* rms        = new double[points];
    for (int i = 0; i < points; i++){
        x[i] = hSignal->GetBinLowEdge(i+1);
        y[i] = hSignal->GetBinContent(i+1);
        //cout << " Bin Center " << hSignal->GetBinLowEdge(2) << endl;
    }
    FindRMS(points, x, y, 0.9*points, mean, rms);
    delete[] x;
    delete[] y;
    delete[] mean;
    cout << "The value of rms[10] = " << rms[10] << endl;

}

The weird thing is that when the program is executed I get a cout from the function with a logical rms, while before the program end I get that rms is 0.

Any idea or advice on why this is happening? The thing is that I have to stick with the function as is, because it belongs to a library I have to stick with...

I thought of changing the function to return a double* instead of void but nothing changed really... Here is the modified function

double* FindRMS(int points, double* signal_x, double* signal_y, int presamples, double* mean, double* rms)
{       
    //double fraction_presamples_RMS = 0.9; // fraction of presamples to calculate RMS
  //Safety condition
  if (presamples>points) {
    printf("\nERROR: Too many presamples!\n\n");
    //return;
  }

  //Main procedure
  (*rms)  =0.;  
  (*mean) =0.;

  for (int i=0; i<presamples; i++) {
    (*mean)+=signal_y[i];
    (*rms)+=pow(signal_y[i],2);
  }

  (*mean)/=presamples;
  (*rms)=sqrt((*rms)/presamples-pow((*mean),2));
    cout << "RMS was found to be : " << (*rms) << endl;

    return rms;

}
Thanos
  • 572
  • 2
  • 7
  • 24
  • The return only matters if you are calling the function and assigning the result of the function to something else. If you just want it to print something `void` is fine. – Eli Sadoff Jun 21 '16 at 15:25
  • 2
    *"First of all, if I understand correctly, double* means that the argument is expected to be dynamically defined"* No, it only means the parameter is a pointer. It doesn't say anything about what it points to. But you should probably be returning the mean and rms, not setting some variables from pointers. – juanchopanza Jun 21 '16 at 15:25
  • @EliSadoff Thanks for your comment! In fact I don't want just to print the output. I want to use the output for later calculations! – Thanos Jun 21 '16 at 15:28
  • @Thanos then you certainly want the method to return `double *`. Having said that, returning pointers is not always the cleanest thing in the world. – Eli Sadoff Jun 21 '16 at 15:29
  • @EliSadoff I can understand that returning a pointer is weird, but what choice do I have? Since I am novice, I don't know how to get around this... But even if I change the function to `double*` I still get `0` for the rms – Thanos Jun 21 '16 at 15:33
  • @juanchopanza I am confused... This definition `double* x = new double[points];` is a dynamic one, right? Or is it the definition of a pointer? – Thanos Jun 21 '16 at 15:35
  • Not really an answer, but why are you using dynamic allocation? I don't see anything here that requires it, and you might see a performance gain by not having to perform repeated dereferences. – Jim V Jun 21 '16 at 15:35
  • 2
    Holy pointers, batman! – Lightness Races in Orbit Jun 21 '16 at 15:35
  • @JimV Thanks for your comment! First of all I was declaring the arrays like this `double x[points], y[points], derivative[points];`. Even though I didn't have any compilation errors, when I was executing my program, it was crashing. I thought that since my arrays have 9millions elements each, the stack wasn't enough. So by dynamically allocating my arrays, my program run. – Thanos Jun 21 '16 at 15:39
  • When you do e.g. `double* x = new double[...]` you are both *defining* the pointer variable `x` and *initializing* it. You initialize the variable to the value returned by the expression `new double[...]`. The expression `new double[...]` allocates memory dynamically. So in one statement you are both defining a variable, initializing it, and dynamically allocating memory. – Some programmer dude Jun 21 '16 at 15:42
  • 1
    @Thanos In that case, I'd use a pre-sized `vector` or `array` and let the container do the memory management for you: `std::vector x(points, 0);`. – Jim V Jun 21 '16 at 15:43
  • @JimV tha thing is that I don't know the size a'priori. So what do I do in this case? – Thanos Jun 21 '16 at 15:48
  • @JoachimPileborg I thought I was dynamically allocating by using `*`... Novice... Thanks a lot for that!!! – Thanos Jun 21 '16 at 15:49
  • Isn't "points" the size that you'll need? – Jim V Jun 21 '16 at 15:53
  • Here's a good write-up on when to use/avoid dynamic allocation: http://stackoverflow.com/a/22146244/2924233 – Jim V Jun 21 '16 at 15:55
  • @JimV Yes... You are absolutely right! Sorry! – Thanos Jun 21 '16 at 15:55
  • I don't think Stallman will approve of software designed to find him. – user4581301 Jun 21 '16 at 17:55

1 Answers1

3

In the function FindRMS you only set *rms which is the same as rms[0]. All other values in the array are uninitialized, which means their values will be indeterminate (and will seem to be random). Reading those uninitialized values will lead to undefined behavior.

Some programmer dude
  • 363,249
  • 31
  • 351
  • 550
  • Thank you very much for your answer! I am confused on the following : The function accepts also a `double* singal_x` and a `double* signal_y`. As far as I understand these are again `x[0]` and `y[0]`. But inside the function there is a loop over the `y[]` array! – Thanos Jun 21 '16 at 15:32
  • @Thanos: The pointer `signal_y` points to the start of the array `y`. – Lightness Races in Orbit Jun 21 '16 at 15:36
  • @Thanos There is a difference between *declaring* a pointer and *dereferencing* a pointer. In C++ many operators and special characters can mean different things in different situation. – Some programmer dude Jun 21 '16 at 15:37
  • @Thanos Also to explain why *dereferencing* the pointer `rms` (i.e. using `*rms` in an expression) is the same as `rms[0]`, it's because for any pointer (let say `rms`) and index `i` the expressions `rms[i]` and `*(rms + i)` are the same. That means `rms[0]` is the same as `*(rms + 0)` and since adding zero to something is a useless operation it can be removed which means `rms[0]` is the same as `*(rms)` which is the same as `*rms`. – Some programmer dude Jun 21 '16 at 15:39
  • @JoachimPileborg I get that! What I don't get is how this line `(*rms)+=pow(signal_y[i],2);` is valid in the sense that since the argument is only `signal_y[0]`(because it has been declared as `double* signal_y`), how does the function know any element in the `signal_y[]` array? – Thanos Jun 21 '16 at 15:45
  • @Thanos You misunderstand the asterisk... In a declaration like when you declare the argument it means "this variable is a pointer", it doesn't mean anything else, it's not the dereference operator, so it doesn't mean `signal_y[0]` in the declaration. It only means `signal_y[0]` if you use the dereference operator in an expression. Like I said, the asterisk means different things depending on situation. In one place it means "declare as a pointer" and in another it means "dereference the pointer". Two different things, in two different situations. – Some programmer dude Jun 21 '16 at 15:48
  • @JoachimPileborg I see what you mean... The function expects a pointer as an argument, therefore it has to be declared as such, so as to use the function. What I don't get is how to parse in the function the array? Is it valid to declare an array of pointers, like I am doing? And if I use as an argument `x` that has been declared as an array of pointers, will the function perform calculations with only the first element `x[0]` – Thanos Jun 21 '16 at 16:02