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

On Tue, Aug 30, 2016 at 09:16:49AM +1000, Dave Chinner wrote:
> 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 <[email protected]>
> > > > ---
> > > >  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....

Agreed. I thought about it today, and it is indeed "safer" and better
understood to use arguments instead of an external variable. I'll fix
it.
Thanks-
Bill


> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> [email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs