[Xorp-hackers] PATCH: Fix uninitialized memory, found by valgrind

[Ben Greear]

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 <greearb at candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com