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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1507162301030.2797@hadrien>
Date:	Thu, 16 Jul 2015 23:06:27 +0200 (CEST)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	"Incarnation P. Lee" <incarnation.p.lee@...look.com>
cc:	oleg.drokin@...el.com, andreas.dilger@...el.com,
	gregkh@...uxfoundation.org, Julia.Lawall@...6.fr,
	HPDD-discuss@...1.01.org, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix one file coding style issus of linux-next-20150708

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

> Signed-off-by: Li Pan (Incarnation P. Lee) <incarnation.p.lee@...look.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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/