On Tue, 13 Dec 2011 20:27:00 -0500 Jeff Squyres <jsquy...@cisco.com> wrote:
> On Dec 13, 2011, at 7:59 PM, Christopher Yeoh wrote: > > > Sorry, late to the discussion. This is a spurious warning caused by > > passing the NULL pointer to the opal free function which is > > actually ok. It was fixed by #2884 - this is why you don't see the > > problem in trunk. > > I have a dim recollection that we talked about this before, but I'm > going to ask again, anyway... > > https://svn.open-mpi.org/trac/ompi/changeset/25430 > > Why are we removing this debug message? It seems like a useful > thing. It might well be legal to call free(NULL), but it seems like > a bad idea to call free if we know the value will be NULL. Its more that the pointer value might be quite validly NULL. For example in this case that is causing the spurious warning a member of a struct is set to NULL initially and may or may not be allocated memory during the lifetime of the struct, and if it is may be cleaned up earlier on by another function and the value set back to NULL. So during the final cleanup of structs that behave this way, we can have this: if (NULL != foo->pointer) free(foo->pointer); In the case where we have a non NULL pointer we end up doing two checks since the free function already does a NULL pointer check at the start before proceeding. Or we can just do: free(foo->pointer) If its in a performance critical area and we think that the NULL pointer case is going to be very common and not calling free would make a difference we can always add a check before calling free. > Did we talk about this before, and I was in the minority for thinking > removing it was a bad idea? I don't think we talked much about this before (maybe you are thinking of the discussion around use of an assert?). There wasn't much discussion on the ticket https://svn.open-mpi.org/trac/ompi/ticket/2884 -- cy...@ozlabs.org
