I was just curious as if I am calling
OBJ_RELEASE(buffer);
buffer = NULL;
on a buffer with an object count different to 1, the buffer is not free'd
but set to NULL. If I call it again the buffer is NULL and the original
buffer will not be free'd. Setting the buffer to NULL seems unnecessary.
I have not seen this as a problem in the code I was just trying to
understand if I have to call only
OBJ_RELEASE(buffer);
or
OBJ_RELEASE(buffer);
buffer = NULL;
and for me the first variant seems to be the correct one.
Adrian
On Thu, Feb 12, 2015 at 04:58:02PM +0900, Gilles Gouaillardet wrote:
> Adrian,
>
> opal_obj_update does not fail or success, it returns the new
> obj_reference_count.
>
>
> can you point to one specific location in the code where you think it is
> wrong ?
>
> OBJ_RELEASE(buffer)
> buffer = NULL;
>
> could be written as
>
> if (((opal_object_t *)buffer)->obj_reference_count == 1) {
> OBJ_RELEASE(buffer);
> } else {
> buffer = NULL;
> }
>
> that would never ever set buffer to NULL twice, but would be wrong
> since there is no atomicity here
> /* that was for for the "unnecessary" part */
>
> about the "wrong" part, why do you think the else branch is wrong ?
> /* i mean setting a pointer to NULL is not necessarily wrong */
>
> Cheers,
>
> Gilles
>
>
> On 2015/02/12 16:41, Adrian Reber wrote:
> > At many places all over the code I see
> >
> > OBJ_RELEASE(buffer)
> > buffer = NULL;
> >
> > Looking at the definition of OBJ_RELEASE() this seems unnecessary and
> > wrong:
> >
> > #define OBJ_RELEASE(object) \
> > do { \
> > if (0 == opal_obj_update((opal_object_t *) (object), -1)) { \
> > opal_obj_run_destructors((opal_object_t *) (object)); \
> > free(object); \
> > object = NULL; \
> > } \
> > } while (0)
> >
> > The object is set to NULL by the macro and only if the opal_obj_update() was
> > successful. So it seems setting the buffer manually to NULL after
> > OBJ_RELEASE()
> > is unnecessary and if opal_obj_update() failed it also is wrong.
> >
> > Adrian
> > _______________________________________________
> > devel mailing list
> > de...@open-mpi.org
> > Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel
> > Link to this post:
> > http://www.open-mpi.org/community/lists/devel/2015/02/16970.php
>
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Link to this post:
> http://www.open-mpi.org/community/lists/devel/2015/02/16971.php
