Re: [Outreachy kernel] [PATCH v2 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev

[Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>]

On Thu, Apr 02, 2015 at 10:31:04AM +0200, Greg KH wrote:
> On Wed, Apr 01, 2015 at 06:09:35PM +0200, Aya Mahfouz wrote:
> > Static inline functions are preferred over macros. Hence, the function
> > obd_check_dev was introduced. obd_check_dev replaces the macro
> > OBD_CHECK_DEV. All functions that call obd_check_dev store the return
> > values and return them if they represent an error code.
> > 
> > Some of the changes were carried out manually while others were done
> > using coccinelle.
> > 
> > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > ---
> > v2: removed unneeded parenthesis around the variable obd.
> > 
> >  drivers/staging/lustre/lustre/include/obd_class.h | 33 ++++++++++++++++++-----
> >  1 file changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
> > index 5314f6f..ddc212a 100644
> > --- a/drivers/staging/lustre/lustre/include/obd_class.h
> > +++ b/drivers/staging/lustre/lustre/include/obd_class.h
> > @@ -341,6 +341,15 @@ do {							    \
> >  	}						       \
> >  } while (0)
> >  
> > +static inline int obd_check_dev(struct obd_device *obd)
> > +{
> > +	if (!(obd)) {
> 
> Why the extra () here?
>
This was supposed to be fixed. It seems I used the first patch numbers
again. I will resend the patchset.

> 
> > +		CERROR("NULL device\n");
> > +		return -ENODEV;
> > +	}
> > +	return 0;
> > +}
> > +
> >  /* ensure obd_setup and !obd_stopping */
> >  #define OBD_CHECK_DEV_ACTIVE(obd)			       \
> >  do {							    \
> > @@ -594,7 +603,9 @@ static inline int obd_precleanup(struct obd_device *obd,
> >  	int rc;
> >  	DECLARE_LU_VARS(ldt, d);
> >  
> > -	OBD_CHECK_DEV(obd);
> > +	rc = obd_check_dev(obd);
> > +	if (rc)
> > +		return rc;
> >  	ldt = obd->obd_type->typ_lu;
> >  	d = obd->obd_lu_dev;
> >  	if (ldt != NULL && d != NULL) {
> > @@ -620,7 +631,9 @@ static inline int obd_cleanup(struct obd_device *obd)
> >  	int rc;
> >  	DECLARE_LU_VARS(ldt, d);
> >  
> > -	OBD_CHECK_DEV(obd);
> > +	rc = obd_check_dev(obd);
> > +	if (rc)
> > +		return rc;
> >  
> >  	ldt = obd->obd_type->typ_lu;
> >  	d = obd->obd_lu_dev;
> > @@ -668,7 +681,9 @@ obd_process_config(struct obd_device *obd, int datalen, void *data)
> >  	int rc;
> >  	DECLARE_LU_VARS(ldt, d);
> >  
> > -	OBD_CHECK_DEV(obd);
> > +	rc = obd_check_dev(obd);
> > +	if (rc)
> > +		return rc;
> >  
> >  	obd->obd_process_conf = 1;
> >  	ldt = obd->obd_type->typ_lu;
> > @@ -1280,7 +1295,9 @@ static inline int obd_notify(struct obd_device *obd,
> >  {
> >  	int rc;
> >  
> > -	OBD_CHECK_DEV(obd);
> > +	rc = obd_check_dev(obd);
> > +	if (rc)
> > +		return rc;
> >  
> >  	/* the check for async_recov is a complete hack - I'm hereby
> >  	   overloading the meaning to also mean "this was called from
> > @@ -1381,7 +1398,11 @@ static inline int obd_health_check(const struct lu_env *env,
> >  static inline int obd_register_observer(struct obd_device *obd,
> >  					struct obd_device *observer)
> >  {
> > -	OBD_CHECK_DEV(obd);
> > +	int rc;
> > +
> > +	rc = obd_check_dev(obd);
> > +	if (rc)
> > +		return rc;
> >  	down_write(&obd->obd_observer_link_sem);
> >  	if (obd->obd_observer && observer) {
> >  		up_write(&obd->obd_observer_link_sem);
> > @@ -1389,7 +1410,7 @@ static inline int obd_register_observer(struct obd_device *obd,
> >  	}
> >  	obd->obd_observer = observer;
> >  	up_write(&obd->obd_observer_link_sem);
> > -	return 0;
> > +	return rc;
> 
> This change isn't needed in this patch.
> 

You mean to keep it return 0 as is?

> thanks,
> 
> greg k-h

-- 
Kind Regards,
Aya Saif El-yazal Mahfouz