Re: [PATCH] Fix one file coding style issus of linux-next-20150708

[Julia Lawall <julia.lawall@lip6.fr>]

On Thu, 16 Jul 2015, Incarnation P. Lee wrote:

> Signed-off-by: Li Pan (Incarnation P. Lee) <incarnation.p.lee@outlook.com>

The signed off line should match the From line, which should match your
real name (passport, etc).  And the signed off line should go after the
commit message.

>     Fix one file coding sytle issue on linux-next-20150708, including macro

Drop the reference to linux-next.  That is not very useful from one day to
the next.  Normally code doesn't change that fast, so the Linux-next when
you submitted the patch is probably pretty much like the one that exists,
with respect to your specific code, when someone picks up the patch.

> aligned, missing blank lines after variable declarnation and brace missing

Brace missing seems backward from what is done.  There are too many
braces, and you want to remove them.

"macro aligned" is also not very understandable.

This seems like three different changes, so it could be better to make
three patches, in a patch series.

> in one line if structure.
>     This patch can be one part of Eudyotula Task10.

You can put this under the ---.  It is not useful for someone who reads
the patch in the future and wants to understand what was done.

julia

> ---
>  drivers/staging/lustre/lustre/obdclass/cl_page.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> index a7f3032..428c6b2 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> @@ -51,13 +51,13 @@
>  static void cl_page_delete0(const struct lu_env *env, struct cl_page *pg,
>  			    int radix);
>
> -# define PASSERT(env, page, expr)				       \
> -  do {								    \
> -	  if (unlikely(!(expr))) {				      \
> -		  CL_PAGE_DEBUG(D_ERROR, (env), (page), #expr "\n");    \
> -		  LASSERT(0);					   \
> -	  }							     \
> -  } while (0)
> +# define PASSERT(env, page, expr)					   \
> +	do {								   \
> +		if (unlikely(!(expr))) {				   \
> +			CL_PAGE_DEBUG(D_ERROR, (env), (page), #expr "\n"); \
> +			LASSERT(0);					   \
> +		}							   \
> +	} while (0)
>
>  # define PINVRNT(env, page, exp) \
>  	((void)sizeof(env), (void)sizeof(page), (void)sizeof !!(exp))
> @@ -169,6 +169,7 @@ int cl_page_gang_lookup(const struct lu_env *env, struct cl_object *obj,
>  	while ((nr = radix_tree_gang_lookup(&hdr->coh_tree, (void **)pvec,
>  					    idx, CLT_PVEC_SIZE)) > 0) {
>  		int end_of_region = 0;
> +
>  		idx = pvec[nr - 1]->cp_index + 1;
>  		for (i = 0, j = 0; i < nr; ++i) {
>  			page = pvec[i];
> @@ -286,6 +287,7 @@ static struct cl_page *cl_page_alloc(const struct lu_env *env,
>  			GFP_NOFS);
>  	if (page != NULL) {
>  		int result = 0;
> +
>  		atomic_set(&page->cp_ref, 1);
>  		if (type == CPT_CACHEABLE) /* for radix tree */
>  			atomic_inc(&page->cp_ref);
> @@ -372,9 +374,8 @@ static struct cl_page *cl_page_find0(const struct lu_env *env,
>  						       idx) == page));
>  	}
>
> -	if (page != NULL) {
> +	if (page != NULL)
>  		return page;
> -	}
>
>  	/* allocate and initialize cl_page */
>  	page = cl_page_alloc(env, o, idx, vmpage, type);
> @@ -1425,7 +1426,7 @@ void cl_page_clip(const struct lu_env *env, struct cl_page *pg,
>  	CL_PAGE_HEADER(D_TRACE, env, pg, "%d %d\n", from, to);
>  	CL_PAGE_INVOID(env, pg, CL_PAGE_OP(cpo_clip),
>  		       (const struct lu_env *,
> -			const struct cl_page_slice *,int, int),
> +			const struct cl_page_slice *, int, int),
>  		       from, to);
>  }
>  EXPORT_SYMBOL(cl_page_clip);
> --
> 1.9.0
>
>