[LTP] [PATCH] tst_res: Print errno number in addition to error name

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Richard Palethorpe]

Occasionally new error numbers are added to the kernel (maybe by
accident). Currently if we do not know the name of them then we just print
???.

This commit simply always prints the error number to aid with debugging.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---

It appears we are atleast missing ENOTSUPP (524) which is probably returned by
create_timer for some alarm clocks on none x86 arches. This isn't entirely
clear, but what is clear is that it would help to know what the error number
is without using strace.

 lib/tst_test.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 245e287fa..46481ca3f 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -177,7 +177,7 @@ static void print_result(const char *file, const int lineno, int ttype,
 {
 	char buf[1024];
 	char *str = buf;
-	int ret, size = sizeof(buf), ssize;
+	int ret, size = sizeof(buf), ssize, int_errno;
 	const char *str_errno = NULL;
 	const char *res;
 
@@ -205,15 +205,19 @@ static void print_result(const char *file, const int lineno, int ttype,
 		abort();
 	}
 
-	if (ttype & TERRNO)
+	if (ttype & TERRNO) {
 		str_errno = tst_strerrno(errno);
+		int_errno = errno;
+	}
 
-	if (ttype & TTERRNO)
+	if (ttype & TTERRNO) {
 		str_errno = tst_strerrno(TST_ERR);
+		int_errno = TST_ERR;
+	}
 
 	if (ttype & TRERRNO) {
-		ret = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET;
-		str_errno = tst_strerrno(ret);
+		int_errno = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET;
+		str_errno = tst_strerrno(int_errno);
 	}
 
 	ret = snprintf(str, size, "%s:%i: ", file, lineno);
@@ -237,7 +241,7 @@ static void print_result(const char *file, const int lineno, int ttype,
 				"Next message is too long and truncated:");
 	} else if (str_errno) {
 		ssize = size - 2;
-		ret = snprintf(str, size, ": %s", str_errno);
+		ret = snprintf(str, size, ": %s (%d)", str_errno, int_errno);
 		str += MIN(ret, ssize);
 		size -= MIN(ret, ssize);
 		if (ret >= ssize)
-- 
2.22.0

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Li Wang]

I think this patch makes sense. Only have tiny comments in below.

  Acked-by: Li Wang <liwang@redhat.com>

On Mon, 19 Aug 2019 at 21:36, Richard Palethorpe <rpalethorpe@suse.com> wrote:
>
> Occasionally new error numbers are added to the kernel (maybe by
> accident). Currently if we do not know the name of them then we just print
> ???.
>
> This commit simply always prints the error number to aid with debugging.
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>
> It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> create_timer for some alarm clocks on none x86 arches. This isn't entirely
> clear, but what is clear is that it would help to know what the error number
> is without using strace.
>
>  lib/tst_test.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 245e287fa..46481ca3f 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -177,7 +177,7 @@ static void print_result(const char *file, const int lineno, int ttype,
>  {
>         char buf[1024];
>         char *str = buf;
> -       int ret, size = sizeof(buf), ssize;
> +       int ret, size = sizeof(buf), ssize, int_errno;
>         const char *str_errno = NULL;
>         const char *res;
>
> @@ -205,15 +205,19 @@ static void print_result(const char *file, const int lineno, int ttype,
>                 abort();
>         }
>
> -       if (ttype & TERRNO)
> +       if (ttype & TERRNO) {
>                 str_errno = tst_strerrno(errno);
> +               int_errno = errno;

Can we just reverse the order of these two statements for better typesetting?

> +       }
>
> -       if (ttype & TTERRNO)
> +       if (ttype & TTERRNO) {
>                 str_errno = tst_strerrno(TST_ERR);
> +               int_errno = TST_ERR;

And this two.

> +       }
>
>         if (ttype & TRERRNO) {
> -               ret = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET;
> -               str_errno = tst_strerrno(ret);
> +               int_errno = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET;
> +               str_errno = tst_strerrno(int_errno);
>         }
>
>         ret = snprintf(str, size, "%s:%i: ", file, lineno);
> @@ -237,7 +241,7 @@ static void print_result(const char *file, const int lineno, int ttype,
>                                 "Next message is too long and truncated:");
>         } else if (str_errno) {
>                 ssize = size - 2;
> -               ret = snprintf(str, size, ": %s", str_errno);
> +               ret = snprintf(str, size, ": %s (%d)", str_errno, int_errno);
>                 str += MIN(ret, ssize);
>                 size -= MIN(ret, ssize);
>                 if (ret >= ssize)
> --
> 2.22.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp



-- 
Regards,
Li Wang
Email: wangli.ahau@gmail.com

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Cyril Hrubis]

Hi!
> It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> create_timer for some alarm clocks on none x86 arches. This isn't entirely
> clear, but what is clear is that it would help to know what the error number
> is without using strace.

Can we please also patch the lib/errnos.h so that the ENOTSUPP is
included there as well?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Cyril Hrubis]

Hi!
> > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> > create_timer for some alarm clocks on none x86 arches. This isn't entirely
> > clear, but what is clear is that it would help to know what the error number
> > is without using strace.
> 
> Can we please also patch the lib/errnos.h so that the ENOTSUPP is
> included there as well?

And also patch the timer_create() tests to return TCONF if we got
ENOTSUPP?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Li Wang]

On Tue, Aug 20, 2019 at 5:56 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> > > create_timer for some alarm clocks on none x86 arches. This isn't entirely
> > > clear, but what is clear is that it would help to know what the error number
> > > is without using strace.
> >
> > Can we please also patch the lib/errnos.h so that the ENOTSUPP is
> > included there as well?
>
> And also patch the timer_create() tests to return TCONF if we got
> ENOTSUPP?

I think we shouldn't TCONF this until we get clear the problem.

This failure very probably a bug for kernel:

# grep RTC_CLASS /boot/config-4.18.0-128.el8.aarch64
CONFIG_RTC_CLASS=y

# ./timer_create01
tst_test.c:1104: INFO: Timeout per run is 0h 05m 00s
...
timer_create01.c:87: FAIL: Failed to create timer for
CLOCK_BOOTTIME_ALARM: ??? (524)
timer_create01.c:87: FAIL: Failed to create timer for
CLOCK_REALTIME_ALARM: ??? (524)

# cat kernel/time/alarmtimer.c -n
...
   57  #ifdef CONFIG_RTC_CLASS
...
    72 struct rtc_device *alarmtimer_get_rtcdev(void)
    73 {
    74         unsigned long flags;
    75         struct rtc_device *ret;
    76
    77         spin_lock_irqsave(&rtcdev_lock, flags);
    78         ret = rtcdev;
    79         spin_unlock_irqrestore(&rtcdev_lock, flags);
    80
    81         return ret;
    82 }
    83 EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev);
...
   140 #else
   141 struct rtc_device *alarmtimer_get_rtcdev(void)
   142 {
   143         return NULL;
   144 }
   ...
   670 static int alarm_timer_create(struct k_itimer *new_timer)
   671 {
   672         enum  alarmtimer_type type;
   673
   674         if (!alarmtimer_get_rtcdev())
   675                 return -ENOTSUPP;
   676
   677         if (!capable(CAP_WAKE_ALARM))
   678                 return -EPERM;
   679
   680         type = clock2alarm(new_timer->it_clock);
   681         alarm_init(&new_timer->it.alarm.alarmtimer, type,
alarm_handle_timer);
   682         return 0;
   683 }

-- 
Regards,
Li Wang

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Cyril Hrubis]

Hi!
> > > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> > > > create_timer for some alarm clocks on none x86 arches. This isn't entirely
> > > > clear, but what is clear is that it would help to know what the error number
> > > > is without using strace.
> > >
> > > Can we please also patch the lib/errnos.h so that the ENOTSUPP is
> > > included there as well?
> >
> > And also patch the timer_create() tests to return TCONF if we got
> > ENOTSUPP?
> 
> I think we shouldn't TCONF this until we get clear the problem.

Indeed, if nothing else it should return ENOTSUP/EOPNOTSUPP instead.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Li Wang]

On Tue, Aug 20, 2019 at 5:55 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> > create_timer for some alarm clocks on none x86 arches. This isn't entirely
> > clear, but what is clear is that it would help to know what the error number
> > is without using strace.
>
> Can we please also patch the lib/errnos.h so that the ENOTSUPP is
> included there as well?

Not sure if we should do that since it(from google) says ENOTSUPP is
not intended to used in userland.

But if we just for catching more errors to report in LTP, I also think
it does not hurt anything.

-- 
Regards,
Li Wang

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Cyril Hrubis]

Hi!
> > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> > > create_timer for some alarm clocks on none x86 arches. This isn't entirely
> > > clear, but what is clear is that it would help to know what the error number
> > > is without using strace.
> >
> > Can we please also patch the lib/errnos.h so that the ENOTSUPP is
> > included there as well?
> 
> Not sure if we should do that since it(from google) says ENOTSUPP is
> not intended to used in userland.

Looks like there are two of these ENOTSUP and ENOTSUPP so the syscall
returning ENOTSUPP is a bug itself since it should return ENOTSUP. Also
btw ENOTSUP == EOPNOTSUPP and we do have EOPNOTSUPP in errnos.h.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
>> > > create_timer for some alarm clocks on none x86 arches. This isn't entirely
>> > > clear, but what is clear is that it would help to know what the error number
>> > > is without using strace.
>> >
>> > Can we please also patch the lib/errnos.h so that the ENOTSUPP is
>> > included there as well?
>>
>> Not sure if we should do that since it(from google) says ENOTSUPP is
>> not intended to used in userland.

I suppose we could print a warning/fail if any error value over 512
(ERESTARTSYS) is seen by tst_res(). We can define these error numbers in
user land for debugging purposes.

>
> Looks like there are two of these ENOTSUP and ENOTSUPP so the syscall
> returning ENOTSUPP is a bug itself since it should return ENOTSUP. Also
> btw ENOTSUP == EOPNOTSUPP and we do have EOPNOTSUPP in errnos.h.

I think there is quite a lot of stuff missing. Maybe it is time to
create a script for generating this list?

--
Thank you,
Richard.

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Cyril Hrubis]

Hi!
> >> > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by
> >> > > create_timer for some alarm clocks on none x86 arches. This isn't entirely
> >> > > clear, but what is clear is that it would help to know what the error number
> >> > > is without using strace.
> >> >
> >> > Can we please also patch the lib/errnos.h so that the ENOTSUPP is
> >> > included there as well?
> >>
> >> Not sure if we should do that since it(from google) says ENOTSUPP is
> >> not intended to used in userland.
> 
> I suppose we could print a warning/fail if any error value over 512
> (ERESTARTSYS) is seen by tst_res(). We can define these error numbers in
> user land for debugging purposes.

Sounds good to me, I wouldn't do it a fail, even simple TINFO message
that would say that we got unexpected errno.

And we should btw do that for the tst_strerrno() as well.

> > Looks like there are two of these ENOTSUP and ENOTSUPP so the syscall
> > returning ENOTSUPP is a bug itself since it should return ENOTSUP. Also
> > btw ENOTSUP == EOPNOTSUPP and we do have EOPNOTSUPP in errnos.h.
> 
> I think there is quite a lot of stuff missing. Maybe it is time to
> create a script for generating this list?

We can but I would just commit the resulting header to LTP since the
list will not change frequently or at all.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] tst_res: Print errno number in addition to error name

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz