[openib-general] [PATCH] kDAPL: use Grant's suggestions

Tom Duffy tduffy at sun.com
Thu Jun 23 10:33:35 PDT 2005


On Wed, 2005-06-22 at 23:20 -0700, Grant Grundler wrote:
> On Wed, Jun 22, 2005 at 10:17:09AM -0700, Tom Duffy wrote:
> > This patch removes the function dapl_os_panic() in favor of calling
> > panic() directly.
> 
> just some nits...
> 
> >  	if (!evd)
> > -		dapl_os_panic("NULL == context\n");
> > +		panic("NULL == context\n");
> 
> Isn't this a bit silly?
> 
> >  	async_evd = evd->common.owner_ia->async_error_evd;
> 
> The system is going to panic anyway here if evd is null.
> I don't see any advantage to testing evd if the only action is to panic.
> 
> > -	if (NULL == context) {
> > -		dapl_os_panic("NULL == context\n");
> > -		return;
> > -	}
> > +	if (NULL == context)
> > +		panic("NULL == context\n");
> 
> Would this be better as "BUG_ON(NULL == context)"?

I agree with both points.

Here is a new patch based off of r2689.

Signed-off-by: Tom Duffy <tduffy at sun.com>

Index: linux-kernel/dat-provider/dapl_evd.c
===================================================================
--- linux-kernel/dat-provider/dapl_evd.c	(revision 2689)
+++ linux-kernel/dat-provider/dapl_evd.c	(working copy)
@@ -680,9 +680,6 @@ static void dapl_evd_cq_async_error_call
 		     "dapl_evd_cq_async_error_callback (%p, %p)\n",
 		     cause, context);
 
-	if (!evd)
-		panic("NULL == context\n");
-
 	async_evd = evd->common.owner_ia->async_error_evd;
 
 	status = dapl_evd_post_async_error_event(async_evd,
@@ -706,8 +703,7 @@ void dapl_evd_un_async_error_callback(st
 		     "dapl_evd_un_async_error_callback (%p, %p)\n",
 		     cause, context);
 
-	if (NULL == context)
-		panic("NULL == context\n");
+	BUG_ON(context == NULL);
 
 	async_evd = (struct dapl_evd *)context;
 




More information about the general mailing list