Re: [PATCH] xfsprogs: don't populate fs table with foreign fs paths unless foreign_allowed

[Dave Chinner <david@fromorbit.com>]

On Mon, Aug 29, 2016 at 10:43:49AM -0500, Bill O'Donnell wrote:
> On Mon, Aug 29, 2016 at 08:13:23AM -0700, Darrick J. Wong wrote:
> > On Mon, Aug 29, 2016 at 08:40:12AM -0500, Bill O'Donnell wrote:
> > > Commits b20b6c2 and 29647c8 modified xfs_quota for use on
> > > non-XFS filesystems. One modification in fs_initialise_mounts
> > > (paths.c) resulted in an xfstest fail (xfs/261), due to foreign
> > > fs paths entering the fs table.
> > > 
> > > This patch reverts the behavior in fs_initialise_mounts back
> > > to skip populating the table with foreign paths, unless the
> > > -f flag is thrown in xfs_quota to set foreign_allowed true.
> > > 
> > > Signed-off-by: Bill O'Donnell <billodo@redhat.com>
> > > ---
> > >  libxcmd/paths.c | 5 +++++
> > >  quota/init.c    | 1 -
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libxcmd/paths.c b/libxcmd/paths.c
> > > index 4158688..7375c0e 100644
> > > --- a/libxcmd/paths.c
> > > +++ b/libxcmd/paths.c
> > > @@ -34,6 +34,7 @@ extern char *progname;
> > >  int fs_count;
> > >  struct fs_path *fs_table;
> > >  struct fs_path *fs_path;
> > > +bool foreign_allowed = false;	/* foreign filesystems not allowed (default) */
> > 
> > /me wonders if this would be better as a parameter to
> > fs_table_initialise_mounts() ?
> 
> Hrmm, it could be, but my notion is that keeping it global
> is a bit cleaner than having to add automatics in 4
> function calls.

For some definition of "cleaner" (i.e. less code change), yes.

However, hiding behavioural state in library globals hides the
connection between the program code that sets it and the library
code that uses it. It's better to explicitly pass function behaviour
control parameters as function parameters, that way you can just
follow the call chain to know what is supposed to be happening as
you read the source....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs