On Saturday 25 August 2007 18:12, Brian Barrett wrote:
> > 1. We have logic in ompi_mpi_abort to prevent recursive invocation
> > (ompi_mpi_abort.c:60):
> >
> >      /* Protection for recursive invocation */
> >      if (have_been_invoked) {
> >          return OMPI_SUCCESS;
> >      }
> >      have_been_invoked = true;
>
> This, IMHO, is a wrong thing to do.  The intent of ompi_mpi_abort()
> was that it never returned.  But now it is?  That seems wrong to me.

Recursive or not, if we add __opal_attribute_noreturn__, the compiler (and 
Coverity, as David said in a private mail) then knows, that the function 
won't exit.

Of course, the compiler does also check, that the function indeed does not 
return...
So the patch below should silence coverity (and help the compiler optimize 
some code which calls ompi_mpi_abort).

OK to check in?

With best regards,
Rainer
-- 
----------------------------------------------------------------
Dipl.-Inf. Rainer Keller       http://www.hlrs.de/people/keller
 High Performance Computing       Tel: ++49 (0)711-685 6 5858
   Center Stuttgart (HLRS)           Fax: ++49 (0)711-685 6 5832
 POSTAL:Nobelstrasse 19                 email: kel...@hlrs.de     
 ACTUAL:Allmandring 30, R.O.030            AIM:rusraink
 70550 Stuttgart
Index: mpiruntime.h
===================================================================
--- mpiruntime.h	(Revision 15949)
+++ mpiruntime.h	(Arbeitskopie)
@@ -93,7 +93,7 @@
  * Abort the processes of comm
  */
 int ompi_mpi_abort(struct ompi_communicator_t* comm,
-                   int errcode, bool kill_remote_of_intercomm);
+                   int errcode, bool kill_remote_of_intercomm) __opal_attribute_noreturn__;

 /**
  * Wait for a TotalView-like debugger if asked.
Index: ompi_mpi_abort.c
===================================================================
--- ompi_mpi_abort.c	(Revision 15949)
+++ ompi_mpi_abort.c	(Arbeitskopie)
@@ -59,7 +59,7 @@

     /* Protection for recursive invocation */
     if (have_been_invoked) {
-        return OMPI_SUCCESS;
+        exit (errcode);
     }
     have_been_invoked = true;

@@ -193,6 +193,7 @@

     /* now that we've aborted everyone else, gracefully die. */
     orte_errmgr.error_detected(errcode, NULL);
-    
-    return OMPI_SUCCESS;
+
+    /* WE SHOULD NEVER GET HERE */
+    exit(errcode);
 }

Reply via email to