BUG: uninitialized variable

[asartori86]

in file geotop.cc line 457 we call the function fill_output_vectors (defined in file output.cc line 4782) but we pass as second argument W which is uninitialized. W is used at line 4875 inside an if condition

if (time->JD_plots->nh > 1 && W>0)

[asartori86]

this is the output of clang-tidy checking clang-analyzer-core*

../src/geotop/geotop.cc:457:21: warning: 2nd function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
                    fill_output_vectors(Dt, W, A->E.get(), A->N.get(), A->G.get(), A->W.get(),
                    ^
../src/geotop/geotop.cc:102:9: note: Assuming the condition is false
    if (!argv[1])
        ^
../src/geotop/geotop.cc:102:5: note: Taking false branch
    if (!argv[1])
    ^
../src/geotop/geotop.cc:113:9: note: Taking false branch
        if (wd == "-v"){
        ^
../src/geotop/geotop.cc:121:13: note: Assuming the condition is false
        if (wd.back() != '/')
            ^
../src/geotop/geotop.cc:121:9: note: Taking false branch
        if (wd.back() != '/')
        ^
../src/geotop/geotop.cc:170:5: note: Calling 'time_loop'
    time_loop(adt.get());
    ^
../src/geotop/geotop.cc:195:34: note: 'W' declared without an initial value
    double t, Dt, JD0, JDb, JDe, W, th, th0;
                                 ^
../src/geotop/geotop.cc:209:9: note: Assuming the condition is false
    if (A->P->max_glac_layers>0)
        ^
../src/geotop/geotop.cc:209:5: note: Taking false branch
    if (A->P->max_glac_layers>0)
    ^
../src/geotop/geotop.cc:234:13: note: Taking false branch
            if ( A->I->time > ((*A->P->end_date)(i_sim)- (*A->P->init_date)(i_sim)) *86400. - 1.E-5)
            ^
../src/geotop/geotop.cc:279:21: note: Taking false branch
                    if (t + Dt > A->P->Dt)
                    ^
../src/geotop/geotop.cc:291:29: note: Assuming the condition is false
                        if (A->P->max_glac_layers>0) copy_snowvar3D(A->G->G, G.get());
                            ^
../src/geotop/geotop.cc:291:25: note: Taking false branch
                        if (A->P->max_glac_layers>0) copy_snowvar3D(A->G->G, G.get());
                        ^
../src/geotop/geotop.cc:311:29: note: Assuming the condition is false
                        if (A->P->en_balance == 1)
                            ^
../src/geotop/geotop.cc:311:25: note: Taking false branch
                        if (A->P->en_balance == 1)
                        ^
../src/geotop/geotop.cc:320:29: note: Assuming the condition is false
                        if (A->P->wat_balance == 1 && en == 0)
                            ^
../src/geotop/geotop.cc:320:52: note: Left side of '&&' is false
                        if (A->P->wat_balance == 1 && en == 0)
                                                   ^
../src/geotop/geotop.cc:329:29: note: Left side of '||' is false
                        if (en != 0 || wt != 0)
                            ^
../src/geotop/geotop.cc:329:25: note: Taking false branch
                        if (en != 0 || wt != 0)
                        ^
../src/geotop/geotop.cc:350:38: note: Left side of '&&' is false
                    while ( out == 0 && Dt > A->P->min_Dt );
                                     ^
../src/geotop/geotop.cc:283:21: note: Loop condition is false.  Exiting loop
                    do
                    ^
../src/geotop/geotop.cc:370:25: note: Left side of '||' is false
                    if (en != 0 || wt != 0)
                        ^
../src/geotop/geotop.cc:370:21: note: Taking false branch
                    if (en != 0 || wt != 0)
                    ^
../src/geotop/geotop.cc:397:25: note: Assuming the condition is false
                    if (A->P->state_pixel == 1 && A->P->dUzrun == 1)
                        ^
../src/geotop/geotop.cc:397:48: note: Left side of '&&' is false
                    if (A->P->state_pixel == 1 && A->P->dUzrun == 1)
                                               ^
../src/geotop/geotop.cc:440:25: note: Assuming the condition is false
                    if (A->P->max_glac_layers>0) copy_snowvar3D(G.get(), A->G->G);
                        ^
../src/geotop/geotop.cc:440:21: note: Taking false branch
                    if (A->P->max_glac_layers>0) copy_snowvar3D(G.get(), A->G->G);
                    ^
../src/geotop/geotop.cc:457:21: note: 2nd function call argument is an uninitialized value
                    fill_output_vectors(Dt, W, A->E.get(), A->N.get(), A->G.get(), A->W.get(),
                    ^
../src/geotop/times.cc:386:10: warning: 1st function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
  return convert_JDandYear_dateeur12(JD, year);
         ^
../src/geotop/times.cc:382:3: note: 'JD' declared without an initial value
  double JD;
  ^
../src/geotop/times.cc:385:3: note: Calling 'convert_JDfrom0_JDandYear'
  convert_JDfrom0_JDandYear(JDfrom0, &JD, &year);
  ^
../src/geotop/times.cc:146:3: note: Taking true branch
  if (JDfrom0 < 1)
  ^
../src/geotop/times.cc:385:3: note: Returning from 'convert_JDfrom0_JDandYear'
  convert_JDfrom0_JDandYear(JDfrom0, &JD, &year);
  ^
../src/geotop/times.cc:386:10: note: 1st function call argument is an uninitialized value
  return convert_JDandYear_dateeur12(JD, year);
         ^
../src/geotop/water.balance.cc:191:17: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
    odb[oopnet] = Pnet;
                ^
../src/geotop/water.balance.cc:71:12: note: 'Pnet' declared without an initial value
    double Pnet, loss;
           ^
../src/geotop/water.balance.cc:81:9: note: Assuming the condition is false
    if (adt->P->qin==1)
        ^
../src/geotop/water.balance.cc:81:5: note: Taking false branch
    if (adt->P->qin==1)
    ^
../src/geotop/water.balance.cc:87:9: note: Assuming the condition is false
    if (adt->P->point_sim != 1)  //distributed simulations
        ^
../src/geotop/water.balance.cc:87:5: note: Taking false branch
    if (adt->P->point_sim != 1)  //distributed simulations
    ^
../src/geotop/water.balance.cc:175:19: note: Assuming the condition is false
        for (j=1; j<=adt->P->total_pixel; j++)
                  ^
../src/geotop/water.balance.cc:175:9: note: Loop condition is false. Execution continues on line 185
        for (j=1; j<=adt->P->total_pixel; j++)
        ^
../src/geotop/water.balance.cc:191:17: note: Assigned value is garbage or undefined
    odb[oopnet] = Pnet;
                ^
../src/libraries/fluidturtle/alloc.cc:54:10: warning: Array access (from variable 'm') results in a null pointer dereference [clang-analyzer-core.NullDereference]
  m[nrl] = (float *) malloc((size_t) ((rows * cols + NR_END) * sizeof(float)));
         ^
../src/libraries/fluidturtle/alloc.cc:46:7: note: Assuming 'm' is null
  if (!m) t_error("Allocation failure 1 in matrix()");
      ^
../src/libraries/fluidturtle/alloc.cc:46:3: note: Taking true branch
  if (!m) t_error("Allocation failure 1 in matrix()");
  ^
../src/libraries/fluidturtle/alloc.cc:50:3: note: Null pointer value stored to 'm'
  m -= nrl;
  ^
../src/libraries/fluidturtle/alloc.cc:54:10: note: Array access (from variable 'm') results in a null pointer dereference
  m[nrl] = (float *) malloc((size_t) ((rows * cols + NR_END) * sizeof(float)));

[asartori86]

@yakopuz @ssenoner @ecor, can you propose a meaningful initial value for W?

[yakopuz]

If I understood well, W is the pointer to the big structure WATER defined in struct.geotop.h.
Does the bug happens only in test with no water budger?
Becuase in that case W might simply just not extists.
Otherwise, all the arrays in the structure W should be initalized, either to novalue -9999 or 0.

[ssenoner]

HM... the disadvantage of using single characters for variable is huge.
in this case W comes form energybalance, line 313, and it is unfortunately not WATER but Weight.
probably it should be initialized to 0, this is the value the weight is set in line 86 of Energy.balance.cc
it has something to do more specifically to the fact that something has to be plotted,
I grepped the logfiles for the following Statement:
"Saving plot number"
no match
@yakopuz any idea?
I think we should have a test case which can trigger this code part....

[yakopuz]

Samuel you are true. There are two W a double W and A->W.
It seems to me this W is a kind of "weight" that is used to "weight" with respect to time some variables that should be plotted in certain conditions.
W is initialized to 0 in energy balance and then it changes if some writing options are acrtivated. When W is not 0, then some oputputs are calculate.
Later, those data are written arrays are filled with data with fill output_vectors.

**The problem arises in tests when the keywords EnergyBalance = 1 ? **

Because w is initalized inside EnergyBalance.
In that case we need either initialize W = 0 if the code does not enter at any time EnergyBalance or, may be better
at line 4876 of output.cc :

if (time->JD_plots->nh > 1 && W>0)

be sure also that the keywords EnergyBalance = 1 is activated

Giacomo

[ssenoner]

@yakopuz do you think you can create an example test for this case?