On 12/15/16 3:20 PM, Eric Biggers wrote:
> 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.
Yes, it's unexpected, and needs work, but for a new command it's probably
best to be consistent with the current (odd) framework... :)
Thanks,
-Eric
> 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
