On Tue, 2009-05-19 at 11:07 -0700, Joel Becker wrote: > On Tue, May 19, 2009 at 08:45:36AM +0200, Jim Meyering wrote: > > Fabio M. Di Nitto wrote: > > >> @@ -150,6 +162,10 @@ void corosync_request_shutdown (void) > > >> poll_stop (0); > > >> totempg_finalize (); > > >> coroipcs_ipc_exit (); > > >> + > > >> + /*Remove uidgid_list*/ > > >> + corosync_remove_uidgid_list (); > > > > > > Is there really a need to free this list on exit? we are about to abort > > > anyway and the kernel will take care of that. > > > > > >> + > > >> corosync_exit_error (AIS_DONE_EXIT); > > >> } > > > > "need", maybe not. > > But it is generally good to explicitly free otherwise-leaked memory, > > even just before exit. > > > > The benefit comes when you use a tool like valgrind to find leaks. > > With the explicit free, no leak is reported here. > > If you remove the free, valgrind reports the leak > > and you or multiple people will have to go investigate it. > > Perhaps multiple times. > > > > This becomes especially relevant only once you have > > enough of a regression test suite to exercise such code paths. > > It's even important just to understand what the code is doing. > There's nothing like hunting around for the free of an allocation and > never finding it. > Basically, if you want a particular allocation to be freed by > exit, you mark it such with a comment and never free it, even on a clean > exit. Otherwise, you try to clean up nicely even in exceptional > circumstances.
The comment approach sure would be good for corosync as there are plenty of areas that are not freed on exit. That's also why I didn't really bother with "just one more" kind of thing. Fabio _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
