Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1615816 > unrolled thread

[PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations

Back to article view | Back to linux.kernel

Contents

  [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations Pushkar Jambhlekar <pushkar.iit@gmail.com> - 2017-04-04 11:20 +0200
    Re: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a  blank line after declarations Dan Carpenter <dan.carpenter@oracle.com> - 2017-04-04 11:50 +0200
      Re: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank  line after declarations Pushkar Jambhlekar <pushkar.iit@gmail.com> - 2017-04-04 12:10 +0200
      Re: [lustre-devel] [PATCH] drivers/staging/lustre:  Coding-guideline: Missing a blank line after declarations "Dilger, Andreas" <andreas.dilger@intel.com> - 2017-04-05 12:00 +0200
      RE: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a  blank line after declarations "Hammond, John" <john.hammond@intel.com> - 2017-04-05 15:50 +0200

#1615816 — [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations

Adding a blank line after declaration

Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com>
---
 drivers/staging/lustre/lustre/llite/dcache.c     | 1 +
 drivers/staging/lustre/lustre/llite/namei.c      | 1 +
 drivers/staging/lustre/lustre/llite/symlink.c    | 1 +
 drivers/staging/lustre/lustre/llite/vvp_io.c     | 1 +
 drivers/staging/lustre/lustre/obdclass/cl_page.c | 1 +
 drivers/staging/lustre/lustre/osc/osc_cache.c    | 1 +
 6 files changed, 6 insertions(+)

diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
index 966f580..38f8466 100644
--- a/drivers/staging/lustre/lustre/llite/dcache.c
+++ b/drivers/staging/lustre/lustre/llite/dcache.c
@@ -126,6 +126,7 @@ static int ll_ddelete(const struct dentry *de)
 static int ll_d_init(struct dentry *de)
 {
 	struct ll_dentry_data *lld = kzalloc(sizeof(*lld), GFP_KERNEL);
+
 	if (unlikely(!lld))
 		return -ENOMEM;
 	lld->lld_invalid = 1;
diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index fc17654..d583696 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -434,6 +434,7 @@ struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
 {
 	if (inode) {
 		struct dentry *new = ll_find_alias(inode, de);
+
 		if (new) {
 			d_move(new, de);
 			iput(inode);
diff --git a/drivers/staging/lustre/lustre/llite/symlink.c b/drivers/staging/lustre/lustre/llite/symlink.c
index cd77b55..60aac42 100644
--- a/drivers/staging/lustre/lustre/llite/symlink.c
+++ b/drivers/staging/lustre/lustre/llite/symlink.c
@@ -129,6 +129,7 @@ static const char *ll_get_link(struct dentry *dentry,
 	struct ptlrpc_request *request = NULL;
 	int rc;
 	char *symname = NULL;
+
 	if (!dentry)
 		return ERR_PTR(-ECHILD);
 
diff --git a/drivers/staging/lustre/lustre/llite/vvp_io.c b/drivers/staging/lustre/lustre/llite/vvp_io.c
index eb5d31a..aa31bc0 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_io.c
+++ b/drivers/staging/lustre/lustre/llite/vvp_io.c
@@ -450,6 +450,7 @@ static void vvp_io_advance(const struct lu_env *env,
 {
 	struct cl_object *obj = ios->cis_io->ci_obj;
 	struct vvp_io	 *vio = cl2vvp_io(env, ios);
+
 	CLOBINVRNT(env, obj, vvp_object_invariant(obj));
 
 	vio->vui_tot_count -= nob;
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
index cd9a40c..71fcc4c 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
@@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
 int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
 {
 	struct cl_io *top = cl_io_top((struct cl_io *)io);
+
 	LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
 	return pg->cp_state == CPS_OWNED && pg->cp_owner == top;
 }
diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
index f8c5fc0..c5ccf56 100644
--- a/drivers/staging/lustre/lustre/osc/osc_cache.c
+++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
@@ -898,6 +898,7 @@ int osc_extent_finish(const struct lu_env *env, struct osc_extent *ext,
 		int offset = last_off & ~PAGE_MASK;
 		int count = last_count + (offset & (blocksize - 1));
 		int end = (offset + last_count) & (blocksize - 1);
+
 		if (end)
 			count += blocksize - end;
 
-- 
2.7.4

[toc] | [next] | [standalone]

#1615837 — Re: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations

On Tue, Apr 04, 2017 at 02:45:26PM +0530, Pushkar Jambhlekar wrote:
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> index cd9a40c..71fcc4c 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> @@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
>  int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
>  {
>  	struct cl_io *top = cl_io_top((struct cl_io *)io);
> +
>  	LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
>  	return pg->cp_state == CPS_OWNED && pg->cp_owner == top;
>  }

This is not related to the patch but I don't understand CLOBINVRNT() and
LINVRNT().

# define LINVRNT(exp) LASSERT(exp)
# define LINVRNT(exp) ((void)sizeof !!(exp))

Why do we do the sizeof() instead of just an empty define?  The compiler
calculates the size at compile time and doesn't execute the expression
so it's the same as an empty define so far as I can tell.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1615858 — Re: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations

((void)sizeof !!(exp)) => I believe it is for compiler to unflag
"unused variable" warning.

On Tue, Apr 4, 2017 at 3:08 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Tue, Apr 04, 2017 at 02:45:26PM +0530, Pushkar Jambhlekar wrote:
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> index cd9a40c..71fcc4c 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> @@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
>>  int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
>>  {
>>       struct cl_io *top = cl_io_top((struct cl_io *)io);
>> +
>>       LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
>>       return pg->cp_state == CPS_OWNED && pg->cp_owner == top;
>>  }
>
> This is not related to the patch but I don't understand CLOBINVRNT() and
> LINVRNT().
>
> # define LINVRNT(exp) LASSERT(exp)
> # define LINVRNT(exp) ((void)sizeof !!(exp))
>
> Why do we do the sizeof() instead of just an empty define?  The compiler
> calculates the size at compile time and doesn't execute the expression
> so it's the same as an empty define so far as I can tell.
>
> regards,
> dan carpenter



-- 
Jambhlekar Pushkar Arun

[toc] | [prev] | [next] | [standalone]

#1616773 — Re: [lustre-devel] [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations

On Apr 4, 2017, at 03:38, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> On Tue, Apr 04, 2017 at 02:45:26PM +0530, Pushkar Jambhlekar wrote:
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> index cd9a40c..71fcc4c 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> @@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
>> int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
>> {
>> 	struct cl_io *top = cl_io_top((struct cl_io *)io);
>> +
>> 	LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
>> 	return pg->cp_state == CPS_OWNED && pg->cp_owner == top;
>> }
> 
> This is not related to the patch but I don't understand CLOBINVRNT() and
> LINVRNT().
> 
> # define LINVRNT(exp) LASSERT(exp)
> # define LINVRNT(exp) ((void)sizeof !!(exp))
> 
> Why do we do the sizeof() instead of just an empty define?  The compiler
> calculates the size at compile time and doesn't execute the expression
> so it's the same as an empty define so far as I can tell.

Even though sizeof() is evaluated at compile time and not runtime, it will at
least evaluate the expression "exp" at compile time.  This is useful to avoid
"unused variable" warnings, syntax errors, etc. in that code when the more
expensive LINVRNT() checking is enabled, but is disabled most of the time.

With an empty expression this wouldn't happen at all, and errors may creep in.

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1616947 — RE: [PATCH] drivers/staging/lustre: Coding-guideline: Missing a blank line after declarations

> On Tue, Apr 04, 2017 at 02:45:26PM +0530, Pushkar Jambhlekar wrote:
> > diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > index cd9a40c..71fcc4c 100644
> > --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> > @@ -482,6 +482,7 @@ void cl_page_disown0(const struct lu_env *env,
> > int cl_page_is_owned(const struct cl_page *pg, const struct cl_io *io)
> > {
> >  	struct cl_io *top = cl_io_top((struct cl_io *)io);
> > +
> >  	LINVRNT(cl_object_same(pg->cp_obj, io->ci_obj));
> >  	return pg->cp_state == CPS_OWNED && pg->cp_owner == top;  }
> 
> This is not related to the patch but I don't understand CLOBINVRNT() and
> LINVRNT().
> 
> # define LINVRNT(exp) LASSERT(exp)
> # define LINVRNT(exp) ((void)sizeof !!(exp))
> 
> Why do we do the sizeof() instead of just an empty define?  The compiler
> calculates the size at compile time and doesn't execute the expression so it's the
> same as an empty define so far as I can tell.

This is someone's attempt to avoid the unused variable warnings which would occur
in some places when LINVRNT(exp) is defined as ((void)0).

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web