[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: FW: Bug found in cbr_traffic.cc file in NS



On Mon, 28 Jun 1999, Tom Henderson wrote:

> The documentation is not consistent-- the example in chapter 25
> (packet_size_) is correct.  However, since all of the other agents in
> ns use "packetSize_",

They do? See below; note the bind listing entry for expoo.cc.

> the correct solution seems to be to change the code (and
> documentation) to use "packetSize_" instead of "packet_size_".  I'll
> look into fixing this in a backward compatible manner.

Great! In a related vein, aren't C++ variables generally meant to be
bound to OTcl variables with the same names? It does strike me as a
useful easy-to-remember convention and makes it easier to document
what the variables are and do without tripping up the errant reader
with misstated case, underlines etc... see if you can spot the odd one
out in this extract from loss-monitor.cc in a recent spapshot:

        bind("nlost_", &nlost_);
        bind("npkts_", &npkts_);
        bind("bytes_", &bytes_);
        bind("lastPktTime_", &last_packet_time_); <- Hi! I'm a free clue!
        bind("expected_", &expected_);
        bind("off_rtp_", &off_rtp_);

in general if you take a look with e.g:
grep bind\(\" ~ns/*.cc
_most_ variables are similar either side of the divide, which is the
Helpful Thing The Dumb User Would Expect, at least until he tries
e.g. tracing last_packet_time_ and runs round in circles because
as usual he's forgotten to search for and check the bind statement as
the essential part of his documentation-reading process. (It's worse
than downloading nsDoc.ps anyway on the grounds that the date on the
webpage is probably old; automatically hacking the ps download link
into the first page of the html docs might be sensible, because
that's where the accurate on-web timestamp is.)

I list others I've found below. In particular, it's the odd
one-off oddities that are bugging me, especially when someone who
thinks in C++ is advising someone who thinks in OTcl.

Ideally, you'd have something that scanned code regularly and
automatically flagged these up. Avoiding discrepancies in future would
be good - consistency helps. nsDoc 3.2.2 should recommend this
strongly IMO, and could probably do with a better example than that
old ASRMAgent constructor, where 2 out of 5 examples are just screwy.

cheers,

L.
 
You know, I think I'm starting to appreciate why everyone else claims
programming ns in C++ is easier. No manual variable translation.

And now, the list of things that could be simplified, including a
sublist of things that probably really should be (~/ns/*.cc only):

expoo.cc:       bind_time("burst_time_", &ontime_);
expoo.cc:       bind_time("idle_time_", Offtime_.avgp());
pareto.cc:      bind_time("burst_time_", &ontime_);
pareto.cc:      bind_time("idle_time_", &offtime_);

cbq.cc: bind_bool("okborrow_", &permit_borrowing_);
red.cc: bind_bool("bytes_", &edp_.bytes);           // boolean: use bytes?
red.cc: bind_bool("queue_in_bytes_", &qib_);        // boolean: q in bytes?
red.cc: bind_bool("wait_", &edp_.wait);
red.cc: bind_bool("setbit_", &edp_.setbit);         // mark instead of drop
red.cc: bind_bool("gentle_", &edp_.gentle);         // increase the packet
tcp-fack.cc:    bind_bool("ss-div4_", &ss_div4_);   hmmm, minus sign.
tcp.cc: bind_bool("bugFix_", &bug_fix_);

cbr_traffic.cc: bind("packet_size_", &size_);
expoo.cc:       bind("packet_size_", &size_)
flowmon.cc:             bind("flowid_", &fid_);
ivs.cc: bind("packetSize_", &size_);
loss-monitor.cc   bind("lastPktTime_", &last_packet_time_);
message.cc:     bind("packetSize_", &size_);
mip-reg.cc:     bind("adSize_", &size_);
mip-reg.cc:     bind("ad_lifetime_", &adlftm_);
mip-reg.cc:     bind("home_agent_", &ha_);
mip-reg.cc:     bind("rreqSize_", &size_);
mip-reg.cc:     bind("reg_lifetime_", &reglftm_);
mip.cc: bind("ttl_", &defttl_);
mobilenode.cc:  bind("X_", &X);
mobilenode.cc:  bind("Y_", &Y);
mobilenode.cc:  bind("Z_", &Z);
mobilenode.cc:  bind("speed_", &speed);
pareto.cc:      bind("packet_size_", &size_);
queue.cc:       bind("limit_", &qlim_);
red.cc: bind("thresh_", &edp_.th_min);              // minthresh
red.cc: bind("maxthresh_", &edp_.th_max);           // maxthresh
red.cc: bind("mean_pktsize_", &edp_.mean_pktsize);  // avg pkt size
red.cc: bind("q_weight_", &edp_.q_w);               // for EWMA
red.cc: bind("linterm_", &edp_.max_p_inv);
red.cc: bind("ave_", &edv_.v_ave);                  // average queue
sie
red.cc: bind("prob1_", &edv_.v_prob1);              // dropping
probability
rtp.cc: bind("packetSize_", &size_);
sa.cc:  bind("packetSize_", &size_);
scheduler.cc:   bind("maxslop_", &slop_);
srm-ssm.cc:  bind("group_scope_",&groupScope_);
srm-ssm.cc:  bind("local_scope_",&localScope_);
srm-ssm.cc:  bind("scope_flag_",&scopeFlag_);
srm-ssm.cc:  bind("rep_id_", &repid_);
tcp-asym.cc:/*  bind("exact_srtt_", &t_exact_srtt_);*/
tcp-full.cc:    bind("segsperack_", &segs_per_ack_);
tcp-full.cc:    bind("segsize_", &maxseg_);
tcp-full.cc:    bind("interval_", &delack_interval_);
tcp-session.cc: bind("owndCorr_", &owndCorrection_);
tcp-sink.cc:    bind("packetSize_", &size_);
tcp-sink.cc:    bind("maxSackBlocks_", &max_sack_blocks_); // used only by sack
tcp.cc: bind("window_", &wnd_);
tcp.cc: bind("windowInitOption_", &wnd_init_option_);
tcp.cc: bind("windowOption_", &wnd_option_);
tcp.cc: bind("windowConstant_", &wnd_const_);
tcp.cc: bind("windowThresh_", &wnd_th_);
tcp.cc: bind("tcpTick_", &tcp_tick_);
tcp.cc: bind("packetSize_", &size_);
tcp.cc: bind("seqno_", &curseq_);
tcp.cc: bind("ack_", &highest_ack_);
tcp.cc: bind("rtt_", &t_rtt_);
tcp.cc: bind("srtt_", &t_srtt_);
tcp.cc: bind("rttvar_", &t_rttvar_);
tcp.cc: bind("backoff_", &t_backoff_);
tfrm.cc:        bind("packetSize_", &size_)
udp.cc: bind("packetSize_", &size_);
udp.cc: bind("packetSize_", &size_);


> Thanks for your comments,
> Tom
> 
> On Mon, 28 Jun 1999, Kim, Bong Ho (Bongho) wrote:
> 
> > Hi,
> > 
> > An example of NS document Ch 1. uses 'packetSize_' as a variable to access
> > CBR traffic packet size and Ch 25.3 uses 'packet_size_' as a variable to
> > access CBR traffic packet size.
> > Which one should be corrected ?
> > Currently cbr_traffic.cc file uses 'packet_size_'.
> > 
> > Thanks in advance.
> > 
> > Bong Ho kim 
> > 
> > > -----Original Message-----
> > > From:	Kim, Bong Ho (Bongho) 
> > > Sent:	Monday, June 28, 1999 1:44 PM
> > > To:	'[email protected]'
> > > Subject:	Bug found in cbr_traffic.cc file in NS
> > > 
> > > Hi,
> > > I don't know if this bug is already known. Anyway here is the bug found.
> > > 
> > > To be consistent with NS document:
> > > 
> > > In the file 'cbr_traffic.cc', the following line 
> > > 
> > >       bind("packet_size_",&size_);
> > > 
> > > Should be
> > > 
> > >      bind("packetSize_",&size_);
> > > 
> > > like this.
> > > 
> > > Bye.
> > > 
> > > Bong Ho kim
> > > (732)332-6831
> > > [email protected]

<[email protected]>PGP<http://www.ee.surrey.ac.uk/Personal/L.Wood/>