#5 tclcl-- TracedInt initialization causing warnings

[Tom Henderson]

(this not found in ns-developers archives for some
reason (7/28/05)).

hi,

I finally got tired of ignoring the weird TracedInt
warnings reported by
valgrind when running ns-2 through it so I spent a bit
of time looking
for the source of these warnings.

Most of them seem to come from TracedInt::assign
(tracedvar.h in tclcl)
which does this:
if (val_ != v) {

Here, when the warning is triggered, it is because val_
is not
initialized correctly (most of the time, it seems to be
set to a
reasonable default value of zero but I don't think
anyone would want to
rely on the C++ library to initialize object memory to
zero before
giving it back to you). The reason for the lack of
initialization comes
from the sad fact that there exist a
TracedInt::TracedInt () explicit
constructor which does not initialize val_ to any value
so the object
exists but it does not exist in a coherent initialized
state (It is not
a coherent state because to put it in a coherent state,
you must assign
to it and to assign to it, you need to check the
previous value which is
not initialized so you check for a random value).

The solutions:

1) initialize val_ always to zero by default in
TracedInt::TracedInt ().
I don't like that but it is possible

2) make TracedInt::TracedInt () private to make sure no
one calls it and
make all callers invoke the TracedInt::TracedInt (int
v) constructor
instead. How many explicit callers ? All of the callers
below invoke the
wrong constructor so they would need to be fixed to
provide a default
value.

./adc/salink.h: TracedInt numfl_;
./queue/red.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/rem.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/vq.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/pi.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/gk.h: TracedInt curq_; /* current qlen
seen by arrivals */
./tcp/tfrc.h: TracedInt ndatapack_; // number of
packets sent
./tcp/tcp-sink.h: TracedInt& maxsackblocks() {
return max_sack_blocks_; }
./tcp/tcp-sink.h: TracedInt max_sack_blocks_;
/* used only by sack sinks */
./tcp/tcp.h: TracedInt t_seqno_; /* sequence
number */
./tcp/tcp.h: TracedInt t_rtt_; /* round trip
time */
./tcp/tcp.h: TracedInt t_srtt_; /* smoothed
round-trip time */
./tcp/tcp.h: TracedInt t_rttvar_; /* variance in
round-trip time */
./tcp/tcp.h: TracedInt t_backoff_; /* current
multiplier, 1 if not backed off */
./tcp/tcp.h: TracedInt dupacks_; /* number of
duplicate acks */
./tcp/tcp.h: TracedInt curseq_; /* highest
seqno "produced by app" */
./tcp/tcp.h: TracedInt highest_ack_; /* not frozen
during Fast Recovery */
./tcp/tcp.h: TracedInt ssthresh_; /* slow start
threshold */
./tcp/tcp.h: TracedInt maxseq_; /* used for
Karn algorithm */
./tcp/tcp.h: TracedInt ndatapack_; /* number
of data packets sent */
./tcp/tcp.h: TracedInt ndatabytes_; /* number
of data bytes sent */
./tcp/tcp.h: TracedInt nackpack_; /* number
of ack packets received */
./tcp/tcp.h: TracedInt nrexmit_; /* number
of retransmit timeouts
./tcp/tcp.h: TracedInt nrexmitpack_; /* number
of retransmited packets */
./tcp/tcp.h: TracedInt nrexmitbytes_; /* number
of retransmited bytes */
./tcp/tcp.h: TracedInt necnresponses_; /* number
of times cwnd was reduced
./tcp/tcp.h: TracedInt ncwndcuts_; /*
number of times cwnd was reduced
./tcp/tcp.h: TracedInt singledup_; /* Send on
a single dup ack. */
./sctp/sctp.h: TracedInt tiCwnd; //
trace cwnd for all destinations
./sctp/sctp.h: TracedInt tiErrorCount; //
trace error count for all destinations
./rap/rap.h: TracedInt seqno_; //
Current sequence number
./rap/rap.h: TracedInt sessionLossCount_; // ~
Packets lost in RAP session
./rap/rap.h: TracedInt curseq_; // max
# of pkts sent

Of course, it should be pointed out that this problem
is not inherent to
TracedInt. It is also present in TracedDouble and
TracedVar which
clearly have no reason to have a public void constructor.

How many TracedDouble ?
./adc/estimator.h: TracedDouble avload_;
./adc/estimator.h: TracedDouble measload_;
./adc/param-adc.cc: TracedDouble resv_rate_;
./queue/red.h: TracedDouble v_ave; /* average
queue size */
./queue/red.h: TracedDouble v_prob1; /* prob. of
packet drop before "count". */
./queue/red.h: TracedDouble cur_max_p; //current max_p
./queue/rem.h: TracedDouble v_pl; /* link price */
./queue/rem.h: TracedDouble v_prob; /* prob. of
packet marking. */
./queue/pi.h: TracedDouble v_prob; /* prob. of
packet drop before "count". */
./tcp/tcp-asym.h: TracedDouble t_exact_srtt_;
./tcp/tcp-asym.h:/* TracedDouble avg_win_; */
./tcp/tcp.h: TracedDouble cwnd_; /* current
window */
./tcp/chost.h: TracedDouble ownd_; /* outstanding
data to host */
./tcp/chost.h: TracedDouble owndCorrection_; /*
correction factor to account for dupacks */
./sctp/sctp.h: TracedDouble tdRto; //
trace rto for all destinations
./rap/rap.h: TracedDouble ipg_; //
Inter packet gap
./rap/rap.h: TracedDouble srtt_; //
Smoothened round trip time
./rap/rap.h: TracedDouble timeout_; // Timeout estimate

This leaves you with 53 occurences of objects
initialized in an invalid
state. I am not going to send a patch for either
solution 1) or 2)
because 1) is trivial to do (even though totally evil)
and 2) requires
the author of the patch to know which default value
these objects should
be initialized with.

Is there any hope of anyone caring enough about this to
try to fix it ?

regards,
Mathieu

hi,

I did write my previous email a bit hastily and forgot
to remove the
instance variables for which the correct constructor is
called from the
list show below. Here is the corrected list of
locations to fix:

TracedInt:
----------

./tcp/tcp.h: TracedInt ndatapack_; /* number
of data packets sent */
./tcp/tcp.h: TracedInt ndatabytes_; /* number
of data bytes sent */
./tcp/tcp.h: TracedInt nackpack_; /* number
of ack packets received */
./tcp/tcp.h: TracedInt nrexmitpack_; /* number
of retransmited packets */
./tcp/tcp.h: TracedInt nrexmitbytes_; /* number
of retransmited bytes */
./tcp/tcp.h: TracedInt necnresponses_; /* number
of times cwnd was reduced
./tcp/tcp.h: TracedInt ncwndcuts_; /*
number of times cwnd was reduced
./tcp/tcp.h: TracedInt ncwndcuts1_; /*
number of times cwnd was reduced
./tcp/tcp.h: TracedInt singledup_; /* Send on
a single dup ack. */
./tcp/tcp-sink.h: TracedInt max_sack_blocks_;
/* used only by sack sinks */
./tcp/tfrc.h: TracedInt ndatapack_; // number of
packets sent
./sctp/sctp.h: TracedInt tiCwnd; //
trace cwnd for all destinations
./sctp/sctp.h: TracedInt tiErrorCount; //
trace error count for all destinations
./queue/gk.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/vq.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/rem.h: TracedInt curq_; /* current qlen
seen by arrivals */
./queue/red.h: TracedInt curq_; /* current qlen
seen by arrivals */

TracedDouble:
-------------

in red.h, someone commented out the relevant
constructor. yuk!
./queue/red.h: TracedDouble v_ave; /* average
queue size */
./queue/red.h: TracedDouble v_prob1; /* prob. of
packet drop before "count". */
./queue/red.h: TracedDouble cur_max_p; //current max_p
in chost.h, the variables are initialized but not in
the correct
location (a patch for this one is located there http://www-
sop.inria.fr/dream/personnel/Mathieu.Lacage/ns2/ns-valgrind-1.patch)
./tcp/chost.h: TracedDouble ownd_; /* outstanding
data to host */
./tcp/chost.h: TracedDouble owndCorrection_; /*
correction factor to account for dupacks */
./sctp/sctp.h: TracedDouble tdRto; //
trace rto for all destinations

Or, a total of 23 locations to fix. I might have missed
or added extra
items but the total should not be off by more than 1 or 2.

Mathieu
--