star-bnl / star-sw

Core software for STAR experiment
26 stars 63 forks source link

Variable redefined in inner scope in OnlTools/Jevp/StJevpBuilders/fttBuilder.cxx #431

Closed plexoos closed 1 month ago

plexoos commented 1 year ago

Describe the bug

In OnlTools/Jevp/StJevpBuilders/fttBuilder.cxx variables ix0 and ix1 are redefined inside an if statement, the assigned values do not change the variables defined in the outer scope:

https://github.com/star-bnl/star-sw/blob/e48dc9d0e7bbb7f84776bba64573401ee4dd8277/OnlTools/Jevp/StJevpBuilders/fttBuilder.cxx#L438-L446

A similar pattern can be seen for iy0 and iy1 variables:

https://github.com/star-bnl/star-sw/blob/e48dc9d0e7bbb7f84776bba64573401ee4dd8277/OnlTools/Jevp/StJevpBuilders/fttBuilder.cxx#L458-L466

To Reproduce

Compile a trivial program:

#include <iostream>

int main ()
{
    int ix0 = 1;

    { int ix0 = 2; }
    std::cout << ix0 << "\n";

    { ix0 = 3; }
    std::cout << ix0 << "\n";
}

Expected behavior

It appears the authors intended to modify the values of the variables inside the if statement

plexoos commented 1 year ago

@jml985 git blame shows that you were the last one who touched these lines. Any comment from you?

jml985 commented 1 year ago

Yeah its obviously bug, but not critical for anything. Its on my list to fix

jml985 commented 1 year ago

And its "blamed" on me because I do all the commits for the production Jevp stuff, not necessarily because I changed the code.

fgeurts commented 1 month ago

Hi @jdbrice and @jml985 , Could you let me know if this issue has been resolved?

jml985 commented 1 month ago

Probably not resolved, but I did comment out the code to get rid of the compile warnings and make a pull request. I'm not sure who the maintainer of this code is now, if there is one. It certainly appears as if the author intended to modify the indexes for this portion of the detector and screwed up redefining the variables, however, the code has been in use for several years so presumably it doesn't matter. The worst case scenario is that the online plots show the data shifted by a bin, but I'm not even sure that this code is relevant as it might have been for a protype version of the detector.

sunxuhit commented 1 month ago

Hi Frank, this is certainly beyond my knowledge. Probably Daniel or David should have a look.

fgeurts commented 1 month ago

Thanks Xu for having looked into this. Sent from my iPhoneOn Mar 25, 2024, at 9:01 PM, Xu Sun @.***> wrote: Hi Frank, this is certainly beyond my knowledge. Probably Daniel or David should have a look.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>