Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] Invalid free (btl_openib_endpoint.c, 448) in v1.5
From: Christopher Yeoh (cyeoh_at_[hidden])
Date: 2011-12-14 17:56:54


On Tue, 13 Dec 2011 20:27:00 -0500
Jeff Squyres <jsquyres_at_[hidden]> 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

-- 
cyeoh_at_[hidden]