2014-12-28 05:57:12

by Alexander Kuleshov

[permalink] [raw]
Subject: [PATCH] fs: overlayfs: Fix coding style issues, missing a blank line after declarations

Signed-off-by: Alexander Kuleshov <[email protected]>
---
fs/overlayfs/copy_up.c | 1 +
fs/overlayfs/overlayfs.h | 10 ++++++++++
fs/overlayfs/super.c | 5 +++++
3 files changed, 16 insertions(+)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index ea10a87..593b00d 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -226,6 +226,7 @@ static int ovl_copy_up_locked(struct dentry *workdir, struct dentry *upperdir,

if (S_ISREG(stat->mode)) {
struct path upperpath;
+
ovl_path_upper(dentry, &upperpath);
BUG_ON(upperpath.dentry != NULL);
upperpath.dentry = newdentry;
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 814bed3..6d65e77 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -23,6 +23,7 @@ extern const char *ovl_opaque_xattr;
static inline int ovl_do_rmdir(struct inode *dir, struct dentry *dentry)
{
int err = vfs_rmdir(dir, dentry);
+
pr_debug("rmdir(%pd2) = %i\n", dentry, err);
return err;
}
@@ -30,6 +31,7 @@ static inline int ovl_do_rmdir(struct inode *dir, struct dentry *dentry)
static inline int ovl_do_unlink(struct inode *dir, struct dentry *dentry)
{
int err = vfs_unlink(dir, dentry, NULL);
+
pr_debug("unlink(%pd2) = %i\n", dentry, err);
return err;
}
@@ -38,6 +40,7 @@ static inline int ovl_do_link(struct dentry *old_dentry, struct inode *dir,
struct dentry *new_dentry, bool debug)
{
int err = vfs_link(old_dentry, dir, new_dentry, NULL);
+
if (debug) {
pr_debug("link(%pd2, %pd2) = %i\n",
old_dentry, new_dentry, err);
@@ -49,6 +52,7 @@ static inline int ovl_do_create(struct inode *dir, struct dentry *dentry,
umode_t mode, bool debug)
{
int err = vfs_create(dir, dentry, mode, true);
+
if (debug)
pr_debug("create(%pd2, 0%o) = %i\n", dentry, mode, err);
return err;
@@ -58,6 +62,7 @@ static inline int ovl_do_mkdir(struct inode *dir, struct dentry *dentry,
umode_t mode, bool debug)
{
int err = vfs_mkdir(dir, dentry, mode);
+
if (debug)
pr_debug("mkdir(%pd2, 0%o) = %i\n", dentry, mode, err);
return err;
@@ -67,6 +72,7 @@ static inline int ovl_do_mknod(struct inode *dir, struct dentry *dentry,
umode_t mode, dev_t dev, bool debug)
{
int err = vfs_mknod(dir, dentry, mode, dev);
+
if (debug) {
pr_debug("mknod(%pd2, 0%o, 0%o) = %i\n",
dentry, mode, dev, err);
@@ -78,6 +84,7 @@ static inline int ovl_do_symlink(struct inode *dir, struct dentry *dentry,
const char *oldname, bool debug)
{
int err = vfs_symlink(dir, dentry, oldname);
+
if (debug)
pr_debug("symlink(\"%s\", %pd2) = %i\n", oldname, dentry, err);
return err;
@@ -87,6 +94,7 @@ static inline int ovl_do_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags)
{
int err = vfs_setxattr(dentry, name, value, size, flags);
+
pr_debug("setxattr(%pd2, \"%s\", \"%*s\", 0x%x) = %i\n",
dentry, name, (int) size, (char *) value, flags, err);
return err;
@@ -95,6 +103,7 @@ static inline int ovl_do_setxattr(struct dentry *dentry, const char *name,
static inline int ovl_do_removexattr(struct dentry *dentry, const char *name)
{
int err = vfs_removexattr(dentry, name);
+
pr_debug("removexattr(%pd2, \"%s\") = %i\n", dentry, name, err);
return err;
}
@@ -120,6 +129,7 @@ static inline int ovl_do_rename(struct inode *olddir, struct dentry *olddentry,
static inline int ovl_do_whiteout(struct inode *dir, struct dentry *dentry)
{
int err = vfs_whiteout(dir, dentry);
+
pr_debug("whiteout(%pd2) = %i\n", dentry, err);
return err;
}
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f16d318..3681d2a 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -175,30 +175,35 @@ void ovl_path_lower(struct dentry *dentry, struct path *path)
int ovl_want_write(struct dentry *dentry)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+
return mnt_want_write(ofs->upper_mnt);
}

void ovl_drop_write(struct dentry *dentry)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+
mnt_drop_write(ofs->upper_mnt);
}

struct dentry *ovl_workdir(struct dentry *dentry)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+
return ofs->workdir;
}

bool ovl_dentry_is_opaque(struct dentry *dentry)
{
struct ovl_entry *oe = dentry->d_fsdata;
+
return oe->opaque;
}

void ovl_dentry_set_opaque(struct dentry *dentry, bool opaque)
{
struct ovl_entry *oe = dentry->d_fsdata;
+
oe->opaque = opaque;
}

--
2.2.1.202.g98acd41.dirty


2014-12-29 02:39:43

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] fs: overlayfs: Fix coding style issues, missing a blank line after declarations

On Sun, Dec 28, 2014 at 11:56:53AM +0600, Alexander Kuleshov wrote:
> Signed-off-by: Alexander Kuleshov <[email protected]>
> ---

For the record: anything of that sort against fs/*.c will be flushed down
the toilet where such valuable contributions belong. Don't even bother.

2014-12-29 02:49:56

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] fs: overlayfs: Fix coding style issues, missing a blank line after declarations

On Mon, Dec 29, 2014 at 02:39:39AM +0000, Al Viro wrote:
> On Sun, Dec 28, 2014 at 11:56:53AM +0600, Alexander Kuleshov wrote:
> > Signed-off-by: Alexander Kuleshov <[email protected]>
> > ---
>
> For the record: anything of that sort against fs/*.c will be flushed down
> the toilet where such valuable contributions belong. Don't even bother.

Joe, could you please explain what has driven you to include that into
scripts/checkpatch.pl and open the countless sphincters? Aren't we
getting enough pointless patches as it is?

Al, stongly tempted to try and sneak a patch to checkpatch.pl declaring the
lack of /* See Figure 1 */ within the first 10 lines of a function an offense
against the Gods Of Style. Then sit back and watch the resulting spew...

2014-12-29 03:01:12

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] fs: overlayfs: Fix coding style issues, missing a blank line after declarations

On Mon, 2014-12-29 at 02:49 +0000, Al Viro wrote:
> On Mon, Dec 29, 2014 at 02:39:39AM +0000, Al Viro wrote:
> > On Sun, Dec 28, 2014 at 11:56:53AM +0600, Alexander Kuleshov wrote:
> > > Signed-off-by: Alexander Kuleshov <[email protected]>
> > > ---
> >
> > For the record: anything of that sort against fs/*.c will be flushed down
> > the toilet where such valuable contributions belong. Don't even bother.
>
> Joe, could you please explain what has driven you to include that into
> scripts/checkpatch.pl and open the countless sphincters? Aren't we
> getting enough pointless patches as it is?

I don't care for that style actually.
It was Andrew Morton that wanted it used globally.

I wanted it to be a --strict test and only for
net/ and drivers/net/ where David Miller prefers
that style.

https://lkml.org/lkml/2014/3/6/550

> Al, stongly tempted to try and sneak a patch to checkpatch.pl declaring the
> lack of /* See Figure 1 */ within the first 10 lines of a function an offense
> against the Gods Of Style. Then sit back and watch the resulting spew...

Cool. Awaiting your patch...

2014-12-29 03:38:17

by Hugh Dickins

[permalink] [raw]
Subject: Re: [PATCH] fs: overlayfs: Fix coding style issues, missing a blank line after declarations

On Sun, 28 Dec 2014, Joe Perches wrote:
> On Mon, 2014-12-29 at 02:49 +0000, Al Viro wrote:
> > On Mon, Dec 29, 2014 at 02:39:39AM +0000, Al Viro wrote:
> > > On Sun, Dec 28, 2014 at 11:56:53AM +0600, Alexander Kuleshov wrote:
> > > > Signed-off-by: Alexander Kuleshov <[email protected]>
> > > > ---
> > >
> > > For the record: anything of that sort against fs/*.c will be flushed down
> > > the toilet where such valuable contributions belong. Don't even bother.
> >
> > Joe, could you please explain what has driven you to include that into
> > scripts/checkpatch.pl and open the countless sphincters? Aren't we
> > getting enough pointless patches as it is?
>
> I don't care for that style actually.
> It was Andrew Morton that wanted it used globally.
>
> I wanted it to be a --strict test and only for
> net/ and drivers/net/ where David Miller prefers
> that style.
>
> https://lkml.org/lkml/2014/3/6/550

Although I am guilty of inflicting it on others, just so their patches
keep checkpatch.pl quiet, I have been finding this rule very counter-
productive, and often at odds with long-established good practice.

It makes sense at the head of a substantial function declaring a
collection of variables; but Alexander's patch gives many fine examples
of where it is stupid, and thank you to him for showing it up so well.

In a wrapper function which finds it convenient to declare and assign
a variable before doing a line or two and returning, a common pattern,
a blank line is just distracting hot air to pacify checkpatch.pl.

In a block which needs one extra local variable to do its work, I find
the rule pushes me to move that declaration to the head of the function
instead of keeping it local to the block, just to avoid the unnecessary
blank line.

Please rescind or refine the rule - thanks.

Hugh