[PATCH] Staging: lustre: lmv: Remove unneeded do-while(0)

[PATCH] Staging: lustre: lmv: Remove unneeded do-while(0)

[Bhumika Goyal]

do {} while (0) wrapper in a single statement macro is unneeded.
Also its ok to keep empty #define here to avoid "statement with
no effect" warning.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/staging/lustre/lustre/lmv/lmv_obd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
index 733222c..7a31bd6 100644
--- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
+++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
@@ -2147,7 +2147,7 @@ static void lmv_adjust_dirpages(struct page **pages, int ncfspgs, int nlupgs)
 	LASSERTF(nlupgs == 0, "left = %d", nlupgs);
 }
 #else
-#define lmv_adjust_dirpages(pages, ncfspgs, nlupgs) do {} while (0)
+#define lmv_adjust_dirpages(pages, ncfspgs, nlupgs)
 #endif	/* PAGE_CACHE_SIZE > LU_PAGE_SIZE */
 
 static int lmv_readpage(struct obd_export *exp, struct md_op_data *op_data,
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: lustre: lmv: Remove unneeded do-while(0)

[Greg KH]

On Mon, Feb 15, 2016 at 04:28:19PM +0530, Bhumika Goyal wrote:
> do {} while (0) wrapper in a single statement macro is unneeded.

Are you sure?  You can easly break code by changing this.

> Also its ok to keep empty #define here to avoid "statement with
> no effect" warning.
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
>  drivers/staging/lustre/lustre/lmv/lmv_obd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
> index 733222c..7a31bd6 100644
> --- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
> +++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
> @@ -2147,7 +2147,7 @@ static void lmv_adjust_dirpages(struct page **pages, int ncfspgs, int nlupgs)
>  	LASSERTF(nlupgs == 0, "left = %d", nlupgs);
>  }
>  #else
> -#define lmv_adjust_dirpages(pages, ncfspgs, nlupgs) do {} while (0)
> +#define lmv_adjust_dirpages(pages, ncfspgs, nlupgs)

That's scary, please leave it alone, unless you audited all users of
this call?  Hint, think about if this call was used as the result of an
if() statement?

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH] Staging: lustre: lmv: Remove unneeded do-while(0)

[Bhumika Goyal]

On Tue, Feb 16, 2016 at 10:11 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Feb 15, 2016 at 04:28:19PM +0530, Bhumika Goyal wrote:
>> do {} while (0) wrapper in a single statement macro is unneeded.
>
> Are you sure?  You can easly break code by changing this.
>
>> Also its ok to keep empty #define here to avoid "statement with
>> no effect" warning.
>>
>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>> ---
>>  drivers/staging/lustre/lustre/lmv/lmv_obd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
>> index 733222c..7a31bd6 100644
>> --- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c
>> +++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c
>> @@ -2147,7 +2147,7 @@ static void lmv_adjust_dirpages(struct page **pages, int ncfspgs, int nlupgs)
>>       LASSERTF(nlupgs == 0, "left = %d", nlupgs);
>>  }
>>  #else
>> -#define lmv_adjust_dirpages(pages, ncfspgs, nlupgs) do {} while (0)
>> +#define lmv_adjust_dirpages(pages, ncfspgs, nlupgs)
>
> That's scary, please leave it alone, unless you audited all users of
> this call?  Hint, think about if this call was used as the result of an
> if() statement?
>
> thanks,
>
> greg k-h

 I found out is that if (PAGE_CACHE_SIZE > LU_PAGE_SIZE) then function
lmv_adjust_dirpages is defined with void retun type and it does
something. Otherwise it is defined as macro and does nothing. Also
lmv_adjust_dirpages is called only once in entire kernel and its retun
value is not stored anywhere which implies that the call is made to
function lmv_adjust_dirpages and not the macro. So I guess the macro
can be dropped as it is of no use.

Thanks
Bhumika