Re: [PATCH] selftests: capabilities: convert error output to TAP13 ksft framework

From: Shuah Khan
Date: Fri Jul 28 2017 - 11:41:41 EST


On 07/27/2017 08:13 PM, Andy Lutomirski wrote:
> On Thu, Jul 27, 2017 at 1:32 PM, Shuah Khan <shuah@xxxxxxxxxx> wrote:
>> On 07/27/2017 12:50 PM, Andy Lutomirski wrote:
>>> On Wed, Jul 26, 2017 at 2:18 PM, Shuah Khan <shuahkh@xxxxxxxxxxxxxxx> wrote:
>>>> Convert errx() and err() usage to appropriate TAP13 ksft API.
>>>>
>>>> Signed-off-by: Shuah Khan <shuahkh@xxxxxxxxxxxxxxx>
>>>> ---
>>>> tools/testing/selftests/capabilities/test_execve.c | 105 ++++++++++++---------
>>>> .../testing/selftests/capabilities/validate_cap.c | 9 +-
>>>> 2 files changed, 65 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/capabilities/test_execve.c b/tools/testing/selftests/capabilities/test_execve.c
>>>> index 7c38233292b0..cf6778441381 100644
>>>> --- a/tools/testing/selftests/capabilities/test_execve.c
>>>> +++ b/tools/testing/selftests/capabilities/test_execve.c
>>>> @@ -1,7 +1,6 @@
>>>> #define _GNU_SOURCE
>>>>
>>>> #include <cap-ng.h>
>>>> -#include <err.h>
>>>> #include <linux/capability.h>
>>>> #include <stdbool.h>
>>>> #include <string.h>
>>>> @@ -39,29 +38,32 @@ static void vmaybe_write_file(bool enoent_ok, char *filename, char *fmt, va_list
>>>> int buf_len;
>>>>
>>>> buf_len = vsnprintf(buf, sizeof(buf), fmt, ap);
>>>> - if (buf_len < 0) {
>>>> - err(1, "vsnprintf failed");
>>>> - }
>>>> - if (buf_len >= sizeof(buf)) {
>>>> - errx(1, "vsnprintf output truncated");
>>>> - }
>>>> + if (buf_len < 0)
>>>> + ksft_exit_fail_msg("vsnprintf failed - %s\n", strerror(errno));
>>>
>>> Could this not be a hypothetical ksft_exit_fail_msg_err or similar?
>>> Or a shorter name like ksft_fatal_err()?
>>>
>>>
>>
>> Is there a reason to add _err() suffix?
>>
>> ksft_exit_fail_msg() is a generic routine for a test to exit
>> with a test failure and print a message. The message doesn't
>> necessarily need to be a standard error message such as the
>> one err() or errx() or strerror() generate.
>>
>> In some cases test could fail with a standard error condition,
>> but not always. In that context, it doesn't make sense to add
>> _err suffix. I leveraged this generic function to replace err()
>> and errx() usages adding strerror() not loose the important
>> information.
>
> The idea behind the _err version is to avoid the extra typing to
> report errno. I suppose you could always report errno, but there are
> contexts where errno is meaningless or garbage.
>
>

Thinking about this some more, using strerror() as a replacements does
drop some information compared to what _err() and _errx() provide.

capabilities is the first test I came across that uses err() and errx()
heavily. I am sure there are more that do that. It might be useful to
provide a equivalent ksft_ framework replacement. I will work on it.

thanks,
-- Shuah