Bruce Simpson wrote: > Ben Greear wrote: >> This patch fixes some errors relating to not initializing memory >> properly. I found these by using valgrind. > > A few questions/points: > > * Why is the initializer for TransactionManager::_next_tid required? > This integer key is never exposed outside of TransactionManager, and > the std::map it indexes doesn't make any assumptions about the key > space. Can you provide the valgrind hit? > > * Why is the initializer for IfConfigTransactionManager::_tid_exec > required? This member is only referenced in two places: when it's set > on the pre_commit, and when the operation result callback fires, it > gets passed by value. There are other places in the FEA using the > TransactionManager. Are they also affected/is there coverage? > > * Can you provide the valgrind hits which are fixed by the memset() > calls in io_ip_socket.cc? > > The CMSG macros should notice if a buffer, passed to a socket call, > didn't return any data. If they aren't, that could be a bug elsewhere. > > We really need to understand the problems these fixes address before > taking them. It is normally good practice to clear buffers, when > needed, but it's OK to omit that step for performance if and only if > it doesn't cause stale state to get picked up. Run rtrmgr under valgrind with OSPF (though it's not OSPF related), and you should see these errors.
I don't think any of them are critical, but they make valgrind noisy so you can't see other errors that might be real. At any rate, it isn't clean code to leave member variables un-initialized. It's just asking for weird problems some day with someone starts using the variables differently. The changes are not in any hot path, so they are not going to hurt any performance. Here's my valgrind start command: valgrind --trace-children=yes --log-file=valgrind_xorp_$XORP_FINDER_SERVER_PORT.%p.txt --leak-check=full --track-origins=yes --track-fds=yes xorp_rtrmgr -p $XORP_FINDER_SERVER_PORT -b $CFG_FILE -P $PIDFILE.rtrmgr Thanks, Ben > > cheers, > BMS -- Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com _______________________________________________ Xorp-hackers mailing list [email protected] http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
