Hah, just caught that as well. Commented on the commit on github. Definitely looks wrong.
-Nathan On Thu, Apr 07, 2016 at 05:43:17PM +0000, Dave Goodell (dgoodell) wrote: > [inline] > > On Apr 7, 2016, at 12:53 PM, git...@crest.iu.edu wrote: > > > > This is an automated email from the git hooks/post-receive script. It was > > generated because a ref change was pushed to the repository containing > > the project "open-mpi/ompi". > > > > The branch, master has been updated > > via 92290b94e0584271d6459a6ab5923a04125e23be (commit) > > from 7cdf50533cf940258072f70231a4a456fa73d2f8 (commit) > > > > Those revisions listed above that are new to this repository have > > not appeared on any other notification email; so we list those > > revisions in full, below. > > > > - Log ----------------------------------------------------------------- > > https://github.com/open-mpi/ompi/commit/92290b94e0584271d6459a6ab5923a04125e23be > > > > commit 92290b94e0584271d6459a6ab5923a04125e23be > > Author: Thananon Patinyasakdikul <tpati...@utk.edu> > > Date: Wed Apr 6 14:26:04 2016 -0400 > > > > Fixed Coverity reports 1358014-1358018 (DEADCODE and CHECK_RETURN) > > > > diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.c > > b/ompi/mca/pml/ob1/pml_ob1_recvreq.c > > index 9d1d402..a912bc3 100644 > > --- a/ompi/mca/pml/ob1/pml_ob1_recvreq.c > > +++ b/ompi/mca/pml/ob1/pml_ob1_recvreq.c > > @@ -106,7 +106,7 @@ static int mca_pml_ob1_recv_request_cancel(struct > > ompi_request_t* ompi_request, > > /* The rest should be protected behind the match logic lock */ > > OB1_MATCHING_LOCK(&ob1_comm->matching_lock); > > if( true == request->req_match_received ) { /* way to late to cancel > > this one */ > > - OPAL_THREAD_UNLOCK(&ob1_comm->matching_lock); > > + OB1_MATCHING_LOCK(&ob1_comm->matching_lock); > > I've only taken a cursory look, but this looks very wrong to me. Shouldn't > you be using the "OB1_MATCHING_UNLOCK" macro instead? Doubly locking the > lock will almost certainly lead to sadness. > > > assert( OMPI_ANY_TAG != ompi_request->req_status.MPI_TAG ); /* not > > matched isn't it */ > > return OMPI_SUCCESS; > > } > > diff --git a/opal/mca/btl/tcp/btl_tcp.h b/opal/mca/btl/tcp/btl_tcp.h > > index f2c8917..7e9d726 100644 > > --- a/opal/mca/btl/tcp/btl_tcp.h > > +++ b/opal/mca/btl/tcp/btl_tcp.h > > @@ -96,7 +96,7 @@ extern int mca_btl_tcp_progress_thread_trigger; > > do { \ > > if(0 < mca_btl_tcp_progress_thread_trigger) { \ > > opal_event_t* _event = (opal_event_t*)(event); > > \ > > - opal_fd_write( mca_btl_tcp_pipe_to_progress[1], > > sizeof(opal_event_t*), \ > > + (void) opal_fd_write( mca_btl_tcp_pipe_to_progress[1], > > sizeof(opal_event_t*), \ > > Seems better to capture the return value and at least put an assert() on it > if it fails, though admittedly things are very screwed up if you overrun the > pipe. > > -Dave > > _______________________________________________ > 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/2016/04/18739.php
pgpN0PoPvU3nD.pgp
Description: PGP signature
