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

[Shuah Khan]

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