zernike.h: uninitialized variable compiler warning - bug?

[rjgildea]

Compiling with clang raises the following uninitialized variable warning. Looking at the code this could indicate a potential bug, although I don't know enough about the code to fix it.

https://github.com/cctbx/cctbx_project/blob/master/scitbx/math/zernike.h#L1060

/Users/rjgildea/software/cctbx/modules/cctbx_project/scitbx/math/zernike.h:1060:62: warning: variable 'b' is uninitialized when used here [-Wuninitialized]
                plm[ neg_lm ] = neg_legendre(this_l, this_m, b);
                                                             ^
/Users/rjgildea/software/cctbx/modules/dials_scratch/jbe/scaling_code/scaling_helper.h:110:41: note: in instantiation of member function
      'scitbx::math::zernike::nss_spherical_harmonics<double>::nss_spherical_harmonics' requested here
        nss_spherical_harmonics<double> nsssphe(
                                        ^
/Users/rjgildea/software/cctbx/modules/cctbx_project/scitbx/math/zernike.h:1052:24: note: initialize the variable 'b' to silence this warning
          FloatType a,b,c;
                       ^
                        = 0.0

[rjgildea]

Hopefully @phzwart could comment on this?

[rjgildea]

This change makes the warning go away and the tests still seem to pass. However at the relevant line above, b would always be zero as a,b,c are reinitialized at each iteration of the loop. Should the intialization be outside of the loop? Or should the value of b be determined somehow before it is used?

diff --git a/scitbx/math/zernike.h b/scitbx/math/zernike.h
index f6d318d32..0209f66e2 100644
--- a/scitbx/math/zernike.h
+++ b/scitbx/math/zernike.h
@@ -1049,7 +1049,9 @@ namespace zernike{
         for (int this_m=0; this_m < max_l_; this_m++){
           // get the indices please for this set of indices
           indices = lm_engine_.lut_of_some_indices_in_legendre_recursion_order( this_m );
-          FloatType a,b,c;
+          FloatType a = 0;
+          FloatType b = 0;
+          FloatType c = 0;
           int this_l;
           this_l = lm_indices_[ indices[0] ][0] ; // the n/l l/m correspondence is annoying but a consequence of naming decisions made earlier
           //this_m = lm_indices_[ indices[0] ][1] ;

[phzwart]

This should be fine!

Thanks

P

On 26 March 2018 at 10:12, Richard Gildea notifications@github.com wrote:

This change makes the warning go away and the tests still seem to pass. However at the relevant line above, b would always be zero as a,b,c are reinitialized at each iteration of the loop. Should the intialization be outside of the loop? Or should the value of b be determined somehow before it is used?

diff --git a/scitbx/math/zernike.h b/scitbx/math/zernike.h index f6d318d32..0209f66e2 100644 --- a/scitbx/math/zernike.h +++ b/scitbx/math/zernike.h @@ -1049,7 +1049,9 @@ namespace zernike{ for (int this_m=0; this_m < maxl; this_m++){ // get the indices please for this set of indices indices = lmengine.lut_of_some_indices_in_legendre_recursion_order( this_m );

• FloatType a,b,c;
• FloatType a = 0;
• FloatType b = 0;
• FloatType c = 0; int this_l; this_l = lmindices[ indices[0] ][0] ; // the n/l l/m correspondence is annoying but a consequence of naming decisions made earlier //this_m = lmindices[ indices[0] ][1] ;

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cctbx/cctbx_project/issues/155#issuecomment-376241317, or mute the thread https://github.com/notifications/unsubscribe-auth/AOyCE94zms4-ZcDEBlTe1k2HzQFj6cviks5tiSFjgaJpZM4S7gX3 .

--

P.H. Zwart Staff Scientist Molecular Biophysics and Integrated Bioimaging & Center for Advanced Mathematics for Energy Research Applications Lawrence Berkeley National Laboratories 1 Cyclotron Road, Berkeley, CA-94703, USA Cell: 510 289 9246

PHENIX: http://www.phenix-online.org CAMERA: http://camera.lbl.gov/

[rjgildea]

So if b always is zero at this particular line, would this change be better?

diff --git a/scitbx/math/zernike.h b/scitbx/math/zernike.h
index f6d318d32..2b568058b 100644
--- a/scitbx/math/zernike.h
+++ b/scitbx/math/zernike.h
@@ -1057,7 +1057,7 @@ namespace zernike{
           plm[ indices[0] ] = a;
           if (this_m>0){
                 int neg_lm = lm_engine_.find_lm( this_l , -this_m );
-                plm[ neg_lm ] = neg_legendre(this_l, this_m, b);
+                plm[ neg_lm ] = neg_legendre(this_l, this_m, 0.0);
           }
           // make sure we can go further!
           if (indices.size() > 1){