Bug 1983 – FlowMonitor returns containers copies instead of references

Bug 1983 - FlowMonitor returns containers copies instead of references


Summary:	FlowMonitor returns containers copies instead of references

Status:	RESOLVED FIXED

Product:	ns-3
Classification:	Unclassified
Component:	python bindings
Version:	ns-3-dev
Hardware:	All All

Importance:	P5 normal
Assigned To:	Gustavo J. A. M. Carneiro

URL:
Keywords:

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2014-09-11 13:36 UTC by Tommaso Pecorella
Modified:	2014-09-30 09:33 UTC (History)
CC List:	2 users (show)

See Also:

Attachments
patch (5.62 KB, patch) 2014-09-11 13:54 UTC, Tommaso Pecorella	Details \| Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tommaso Pecorella 2014-09-11 13:36:14 UTC

The following line seems to be right, but it leads to an infinite loop:

std::map<FlowId, FlowMonitor::FlowStats>::iterator j;
for (j = monitor->GetFlowStats().begin(); j != monitor->GetFlowStats().end(); j++)
{...}

GetFlowStats() should return a const reference to the inner container, not a copy of it. Otherwise the two unction calls will return different containers, with different iterators, and the loop will behave in funny way.

Comment 1 Tommaso Pecorella 2014-09-11 13:54:33 UTC

Created attachment 1874 [details]
patch

Added containers typedefs and return containers const references.

Comment 2 Tommaso Pecorella 2014-09-19 16:04:08 UTC

changeset:   10962:8beb54fffadc

Comment 3 Tom Henderson 2014-09-24 00:52:22 UTC

The new API is not working with pybindgen.

Old API was:

  std::map<FlowId, FlowStats> GetFlowStats () const;

Binding:

   ## flow-monitor.h (module 'flow-monitor'): std::map<unsigned int, ns3::FlowMonitor::FlowStats, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, ns3::FlowMonitor::FlowStats> > > ns3::FlowMonitor::GetFlowStats() const [member function]
    cls.add_method('GetFlowStats',
                   'std::map< unsigned int, ns3::FlowMonitor::FlowStats >',
                   [],
                   is_const=True)

This is the new API:

  typedef std::map<FlowId, FlowStats> FlowStatsContainer;
  const FlowStatsContainer& GetFlowStats () const;

and its binding:

    ## flow-monitor.h (module 'flow-monitor'): std::map<unsigned int, ns3::FlowMonitor::FlowStats, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, ns3::FlowMonitor::FlowStats> > > ns3::FlowMonitor::GetFlowStats() const [member function]
    cls.add_method('GetFlowStats',
                   'std::map< unsigned int, ns3::FlowMonitor::FlowStats > &',
                   [],
                   is_const=True)

However, the build log reports:

DEBUG:pybindgen.typehandlers:TypeMatcher.lookup('std::map< unsigned int, ns3::FlowMonitor::FlowStats > const &')
DEBUG:pybindgen.typehandlers:try to lookup type handler for 'std::map< unsigned int, ns3::FlowMonitor::FlowStats > const &' => failure
'std::map< unsigned int, ns3::FlowMonitor::FlowStats > const &' did not match
.../ns-3-dev/src/flow-monitor/bindings/modulegen__gcc_LP64.py:4251: Warning: exception TypeLookupError(['std::map< unsigned int, ns3::FlowMonitor::FlowStats > &'],) in wrapper public: retval? ???::GetFlowStats (params?) const;
  is_const=True)


This doesn't seem limited to this method; GetAllProbes() also does not have a binding.  Seems like pybindgen may not handle returning const references to containers.

Comment 4 Gustavo J. A. M. Carneiro 2014-09-26 08:02:45 UTC

I should be able to tweak pybindgen to handle these cases.  Of course, PBG will have to copy the value anyway, but you cannot escape that in Python.

Comment 5 Tom Henderson 2014-09-30 09:33:27 UTC

fixed in changeset 6015cf72891f