Logical error of a function passing a pointer to an array

ROOT
1

Hello,
I am trying to declare a function that passes a pointer to an array; it is shown below:

Double_t* H(Double_t lambda[voxels],Double_t s2s[sie/2],std::array<std::array<Double_t,voxels>,(sie/2)> L){


Double_t sum[sie/2],gradient[voxels];


for(int i=0;i<sie/2;i++){

sum[i]=0;

for(int j=0;j<voxels;j++){

sum[i]+=L[i][j]*lambda[j];

}

 } //pass


for(int j=0;j<voxels;j++){

gradient[j]=0;

for(int i=0;i<sie/2;i++){

  if(sum[i]!=0){
gradient[j]+=L[i][j]*(sum[i]-s2s[i])/pow(sum[i],2);
  }
}

 }//pass

Double_t f=0;

for(int i=0;i<sie/2;i++){

  if(sum[i]!=0){
    f+=(s2s[i]/sum[i])  + log(sum[i]);
  }
 }//pass

Double_t norm=0;

for(int j=0;j<voxels;j++){

norm+=pow(gradient[j],2);

 }
ar[0]=f;
ar[1]=norm;
for(int j=2;j<voxels+2;j++){

ar[j]=gradient[j-2];

 }//pass
return ar;

}

and then later in my main snippet, I declare two pointer variables that will take value from this function.
First I declare a variable named lambda and assign to it the value returned by the function

Double_t *lambda= new Double_t[voxels+2];
lambda=H(a,s2s,L);  //works

Then I declare another variable l1 and assign it a value returned by the function, but using different values of parameters

Double_t* l1= new Double_t[voxels+2]; 
 l1=H(diff,s2s,L);

The problem is, once I assign the value to l1, that same value is automatically stored in lambda also. What should I do to fix it, so that lambda retains its original value, even after the initialization of l1??

2

“H” always returns “ar” so “l1 = lambda = ar” (note: you set pointer values).

BTW. When you post “source code” or “output” here, do remember to enclose them into two lines which contain just three characters ``` (see how your post has been edited above).

3

Got it. Thank you. I’ll make sure of that next time.

Also, is there a way around to achieving the expected output( keeping lambda unchanged even while modifying l1)??

4

The parameter list looks strange. What exactly are you trying to achieve? Even if voxels and sie are compile time constants, the value is ignored in the parameter! You could as well write:
H(Double_t lambda[], Double_t s2s[], ...)
or even
H(Double_t *lambda, Double_t *s2s, ...)

Arrays decay to pointer in function arguments. If you really want C style array passing, you should pass the corresponding length as separate argument!
When using std::vector or std::array, argument passing works the way you would expect it. Please note that you are copying L! Is that what you really want to do?

Also, where does “ar” come from? It seems to be a global variable. Don’t do that. I don’t exactly understand what you are trying to achieve. Maybe pass “ar” as argument so that you can change it? Most probably the best solution would be to return a std::vector<Double_t> instead of a pointer to double!

5

Try:

Double_t *lambda = new Double_t[(voxels + 2)];
memcpy(lambda, H(a, s2s, L), (sizeof(Double_t) * (voxels + 2)));
// ...
Double_t *l1 = new Double_t[(voxels + 2)];
memcpy(l1, H(diff, s2s, L), (sizeof(Double_t) * (voxels + 2)));
// ...
delete [] l1; // cleanup
delete [] lambda; // cleanup
6

Sorry, I strongly disagree here. While this will work, I would try to avoid memcpy, new and delete of arrays. Chances are high you create a memory leak somehow. Dhruv_Desai1 is already using std::array, i.e. C++. In modern C++, you would never write code like that.

Steps to improve Dhruv_Desai1’s code:

  • function should not depend on external constants/parameters
  • add const for all const parameters
  • function should return a vector (or, if you really want to, a std::unique_ptr<double[]>)
7

Works for now! Thank you very much

8

Sure, I’ll make changes as you said. Thank you very much.

9

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.