On Mon, Aug 22, 2016 at 11:47:42AM +1000, Dave Chinner wrote:
> On Tue, Aug 16, 2016 at 09:16:36AM -0500, Bill O'Donnell wrote:
> > This allows xfs_quota to be used on ext4 for project quota testing
> > in xfstests.
> >
> > This patch was originally submitted by Dave Chinner
> > (http://oss.sgi.com/archives/xfs/2016-02/msg00131.html)
> >
> > Resubmitting with the following change:
> > quota/init.c: correct logic error in loop contained in init_args_command()
> > function (lines 85-91).
>
> What logic error?
In your original patch, in init_args_command():
do {
fs_path = &fs_table[index++];
- } while ((fs_path->fs_flags & FS_PROJECT_PATH) && index < fs_count);
+ if (fs_path->fs_flags & FS_PROJECT_PATH)
+ continue;
+ if (!foreign_allowed && (fs_path->fs_flags & FS_FOREIGN))
+ continue;
+ } while (index < fs_count);
The loop should break out, when (fs_path->fs_flags & FS_PROJECT_PATH) is false,
but instead moves onto the next test (and then back to the top). See in the
original while statement, the loop stops when the false condition occurs, that
is,
((fs_path->fs_flags & FS_PROJECT_PATH) && index < fs_count) == False.
My commit message was completely terse, sorry. I'll clarify it in v3.
Thanks-
Bill
>
> Commit messages like this really don't tell the reader anything
> about what is different the original patch. I've had to go archive
> spelunking to work out what is different, and I'm still not sure
> what the logic error you fixed is....
>
> And, FWIW, whilst spelunking, I noticed that Eric's last review
> comments on my original patch:
>
> Looks ok, but now with the new option:
>
> 1) needs a manpage update
> 2) usage() should be updated to include -f
>
> 3) and I just noticed,
>
> _("foreign mount active, %s command is for XFS filesystems only\n"),
>
> seems kind of unclear; maybe just
>
> _("%s command is for XFS filesystems only\n"),
>
> have not been addressed by this update.
>
> Can you please add these these changes, update the commit message
> and resubmit?
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
|