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

[James Lentini]

Committed in revision 2691.

On Thu, 23 Jun 2005, Tom Duffy wrote:

tduffy> On Wed, 2005-06-22 at 23:20 -0700, Grant Grundler wrote:
tduffy> > On Wed, Jun 22, 2005 at 10:17:09AM -0700, Tom Duffy wrote:
tduffy> > > This patch removes the function dapl_os_panic() in favor of calling
tduffy> > > panic() directly.
tduffy> > 
tduffy> > just some nits...
tduffy> > 
tduffy> > >  	if (!evd)
tduffy> > > -		dapl_os_panic("NULL == context\n");
tduffy> > > +		panic("NULL == context\n");
tduffy> > 
tduffy> > Isn't this a bit silly?
tduffy> > 
tduffy> > >  	async_evd = evd->common.owner_ia->async_error_evd;
tduffy> > 
tduffy> > The system is going to panic anyway here if evd is null.
tduffy> > I don't see any advantage to testing evd if the only action is to panic.
tduffy> > 
tduffy> > > -	if (NULL == context) {
tduffy> > > -		dapl_os_panic("NULL == context\n");
tduffy> > > -		return;
tduffy> > > -	}
tduffy> > > +	if (NULL == context)
tduffy> > > +		panic("NULL == context\n");
tduffy> > 
tduffy> > Would this be better as "BUG_ON(NULL == context)"?
tduffy> 
tduffy> I agree with both points.
tduffy> 
tduffy> Here is a new patch based off of r2689.
tduffy> 
tduffy> Signed-off-by: Tom Duffy <tduffy at sun.com>
tduffy> 
tduffy> Index: linux-kernel/dat-provider/dapl_evd.c
tduffy> ===================================================================
tduffy> --- linux-kernel/dat-provider/dapl_evd.c	(revision 2689)
tduffy> +++ linux-kernel/dat-provider/dapl_evd.c	(working copy)
tduffy> @@ -680,9 +680,6 @@ static void dapl_evd_cq_async_error_call
tduffy>  		     "dapl_evd_cq_async_error_callback (%p, %p)\n",
tduffy>  		     cause, context);
tduffy>  
tduffy> -	if (!evd)
tduffy> -		panic("NULL == context\n");
tduffy> -
tduffy>  	async_evd = evd->common.owner_ia->async_error_evd;
tduffy>  
tduffy>  	status = dapl_evd_post_async_error_event(async_evd,
tduffy> @@ -706,8 +703,7 @@ void dapl_evd_un_async_error_callback(st
tduffy>  		     "dapl_evd_un_async_error_callback (%p, %p)\n",
tduffy>  		     cause, context);
tduffy>  
tduffy> -	if (NULL == context)
tduffy> -		panic("NULL == context\n");
tduffy> +	BUG_ON(context == NULL);
tduffy>  
tduffy>  	async_evd = (struct dapl_evd *)context;
tduffy>  
tduffy> 
tduffy> _______________________________________________
tduffy> openib-general mailing list
tduffy> openib-general at openib.org
tduffy> http://openib.org/mailman/listinfo/openib-general
tduffy> 
tduffy> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
tduffy>