[Ns-bugs] [Bug 273] New: method arguments of type Ptr<Packet> are way too fragile in public API

[bugzilla-daemon@nsnam-www.ece.gatech.edu]

http://www.nsnam.org/bugzilla/show_bug.cgi?id=273

           Summary: method arguments of type Ptr<Packet> are way too fragile
                    in public API
           Product: ns-3
           Version: pre-release
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P3
         Component: node module
        AssignedTo: ns-bugs at isi.edu
        ReportedBy: mathieu.lacage at sophia.inria.fr


This is a really old never-ending complaint of mine that passing around pointer
ownership is _bad_ but I am getting tired of spotting bugs such as this one in
csma-net-device.cc:

      m_rxTrace (packet);
      if (!m_promiscRxCallback.IsNull ())
        {
          m_promiscRxCallback (this, packet, 0, GetBroadcast (), GetAddress (),
PACKET_HOST);
        }
      m_rxCallback (this, packet, 0, GetBroadcast ());

There is at least another occurence of this bug in this file: this code assumes
that m_promisxRxCallback does not take ownership of the input packet pointer
which is wrong because the signature of NetDevice::PromiscReceiveCallback and
NetDevice::ReceiveCallback clearly state that the callee takes ownership of the
input packet pointer because it is of type Ptr<Packet>. The callee code is
actually really taking ownership of the packet so, we did not get any bug
report yet but they will eventually come when we use bridging with multiple
interfaces and ipv4.

I found out about this bug when trying to implement the same functionality in
the wifi net device: I was trying to get this right and make the needed packet
copies in the caller WifiNetDevice when I noticed that it was incredibly
painful and I wondered if the csma code had got it right. It obviously had not.

Proposal: use Ptr<const Packet> instead of Ptr<Packet> in:
  - NetDevice::ReceiveCallback
  - NetDevice::PromiscReceiveCallback
  - Node::ProtocolHandler

I think that it would also make sense to avoid the same problem in the downward
direction by also changing the following methods to use Ptr<const Packet>:
  - NetDevice::Send
  - NetDevice::SendFrom

Please, can we avoid another endless argument about this and, for once, do the
right thing, that is, stay on the safe side of this crap and stop passing
around packet pointer ownership across _very_ public API ?

patch coming.


-- 
Configure bugmail: http://www.nsnam.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.