[Xorp-hackers] valgrind: selector.cc: Reading free'd memory

[Bruce Simpson]

Ben Greear wrote:
> The problem is that a method called by an object can cause that object
> to be deleted, and when that method continues, it is accessing deleted
> memory.

I defer to the wisdom of folk like Scott Meyers in this sort of 
situation... however I'm glad the problem is read access, which limits 
the scope of error to logic, rather than heap corruption. Caveat: I 
didn't write it, I just have to work with it :-)

The situation in win_dispatcher.cc, which I did write, is totally 
different. Whilst a std::map may not be as efficient in memory space as 
a vector, the scope for error is a bit more limited, even if the 
container isn't intrusive.

>
> I have some more regression tests to run and I'm finding bugs everywhere
> I look (in xorp and my own code too for that matter).  My plan is to run
> under valgrind some more, fix easy valgrind bugs and all possible 
> crashes.

Sounds good. We are grateful for any and all bug hunting work which you 
might engage in.

There are a number of valgrind hits to be found, I'm sure. Static 
analyzers like cppcheck can only go so far. It might be interesting to 
fire something like Coverity up, although that involves signing a 
license agreement.

>
> Then, run under oprofile and try to optimize things.  OLSR burns 100% CPU
> during re-configure, and xorpsh seems fairly expensive to launch as well.

If OLSR is racing during a reconfigure, profiling data would be 
interesting to see. I can say off the top of my head that most OLSR 
reconfiguration ops will cause the link state to be lost or 
recalculated. If there is low hanging race fruit to be fixed, bring it on.

One of the things Pavlin identified as a possible TODO item is to 
refactor the routing processes to use a transaction model at the RPC 
(XRL or Thrift) layer -- this would greatly simplify the Router Manager, 
as it can then use commit/rollback directly, rather than trying to 
emulate it in the config tree. However it does push some of the 
complexity of holding configuration state to the processes themselves.

>
> That reminds me:  What is the plan when 'corporate' releases their
> code to customers?  Since it's GPL, we will then have access to the 
> source.

As far as I know, not all of the code in the corporate branch is under 
the GPL, some of it is subject to NDA -- so no, not all of that source 
would be publicly visible.

Obviously the parts which are under GPL, are already available in the 
public tree, however it's up to XORP, Inc. to make changes to GPLed code 
available publicly.

I'm not responsible for compliance, so I can't speak for whether or not 
that really is the case. It seems reasonable that release would be on a 
best-effort basis.

> Do you plan to merge their tree with the public SVN tree at that point?

I had sketched out a schema for this with JT, but as he is no longer 
responsible for SVN inside the company, this would be subject to 
discussion again in the future.

The plan has been to try to preserve similar source tree layouts to make 
this easier. As corporate is obviously using a private SVN server, the 
revision numbers change, and a merge process is then required. This can 
be automated up to a point, but would be mostly manual.

It happens anyway whenever we cross a versioning system boundary, so the 
change involved is largely procedural -- it's down to how the tools are 
deployed to facilitate code sharing. SVN mergeinfo is properly supported 
since Subversion 1.5, so it shouldn't be a big deal tool-wise.

cheers,
BMS