On Thu, Dec 15, 2016 at 01:40:12PM -0600, Eric Sandeen wrote:
> On 12/14/16 10:19 PM, Eric Sandeen wrote:
> >> +
> >> +static int
> >> +get_encpolicy_f(int argc, char **argv)
> >> +{
> >> + struct fscrypt_policy policy;
> >> +
> >> + if (ioctl(file->fd, FS_IOC_GET_ENCRYPTION_POLICY, &policy) < 0) {
> >> + fprintf(stderr, "%s: failed to get encryption policy: %s\n",
> >> + file->name, strerror(errno));
> >> + return 1;
> > Ok, I need to go look at dave's other patch series to give you guidance
> > on what to return here, or anything else under the foo_f() functions.
> >
> > see [PATCH 0/6] xfs_io: fix up command iteration - I need to see how dave
> > wants that all to work now. Prior to that, anyway, most commands returned
> > 0 even on an error, and possibly set exitcode = 1, but I have to see what
> > that patchset does.
>
> Ok, having looked at dave's patchset, it doesn't really change this yet.
>
> Today, almost every command returns 0 regardless of failure, but sets
> exitcode=1 if a failure occurred. This lets command processing continue,
> but results in an ultimate non-zero exit from the utility.
>
> For argument parsing, failures should almost certainly return 0;
> even command_usage() does this.
>
> For functional failures, see i.e. allocsp_f:
>
> if (xfsctl(file->name, file->fd, XFS_IOC_ALLOCSP64, &segment) < 0) {
> perror("XFS_IOC_ALLOCSP64");
> return 0;
> }
>
> (though that didn't set exitcode...)
>
> This all needs cleanup across xfs_io, but I think these new functions will
> be outliers for now if you are returning 1 in many locations under your
> new foo_f functions. Unless you want commandline processing to stop,
> and for interactive shell to quit, you probably want to switch to returning
> 0 even on errors, (usually after printing a message.)
>
> At some point we should probably clean this up, and possibly make it
> continue for interactive shell, but stop for commandline operation,
> or something along those lines. But that's for another day ...
>
Okay, I guess I'll make them always return 0, and additionally set exitcode=1 if
the actual ioctl failed. I'll send v2 of the patch to address this and your
other comments.
I'm not an expert in xfs_io or xfsprogs, but the way I would have expected it to
work is that commands would return a nonzero value if they fail, and then the
higher level logic would use that value to decide whether to continue on and
what to use as the overall exit status --- probably continue by default, then
exit with failure status overall if any one command failed.
Thanks,
Eric
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
