Re: [PATCH v2 1/5] autofs - improve ioctl sbi checks

[Ian Kent]

On Fri, 2018-11-23 at 15:29 -0800, Andrew Morton wrote:
> On Fri, 23 Nov 2018 18:41:50 +0800 Ian Kent <raven@xxxxxxxxxx> wrote:
> 
> > Al Viro made some suggestions to improve the implementation
> > of commit 0633da48f0 "fix autofs_sbi() does not check super
> > block type".
> > 
> > The check is unnessesary in all cases except for ioctl usage
> > so placing the check in the super block accessor function
> > adds a small overhead to the common case where it isn't
> > needed.
> > 
> > So it's sufficient to do this in the ioctl code only.
> > 
> > Also the check in the ioctl code is needlessly complex.
> > 
> > ...
> > 
> > --- a/fs/autofs/dev-ioctl.c
> > +++ b/fs/autofs/dev-ioctl.c
> > @@ -14,6 +14,8 @@
> >  
> >  #include "autofs_i.h"
> >  
> > +extern struct file_system_type autofs_fs_type;
> > +
> >  /*
> >   * This module implements an interface for routing autofs ioctl control
> >   * commands via a miscellaneous device file.
> 
> It's naughty to declare externs in C files, for various reasons.  Is
> this OK?

OK, I understand the reasoning I guess.

The change below is fine.

Thanks, Ian.

> 
> --- a/fs/autofs/autofs_i.h~autofs-improve-ioctl-sbi-checks-fix
> +++ a/fs/autofs/autofs_i.h
> @@ -42,6 +42,8 @@
>  #endif
>  #define pr_fmt(fmt) KBUILD_MODNAME ":pid:%d:%s: " fmt, current->pid, __func__
>  
> +extern struct file_system_type autofs_fs_type;
> +
>  /*
>   * Unified info structure.  This is pointed to by both the dentry and
>   * inode structures.  Each file in the filesystem has an instance of this
> --- a/fs/autofs/dev-ioctl.c~autofs-improve-ioctl-sbi-checks-fix
> +++ a/fs/autofs/dev-ioctl.c
> @@ -14,8 +14,6 @@
>  
>  #include "autofs_i.h"
>  
> -extern struct file_system_type autofs_fs_type;
> -
>  /*
>   * This module implements an interface for routing autofs ioctl control
>   * commands via a miscellaneous device file.
> _
>