[LTP] new ltp lib doesn't print actual error

[LTP] new ltp lib doesn't print actual error

[Alexey Kodanev]

Hi,

When running a test with the new LTP lib, noticed that if some error 
occurs in a library (tst_tmpdir.c), it'll print the same message:

TMPDIR=/usr ./creat01
tst_tmpdir.c:155: BROK: Non-NULL cleanup in newlib!

but should be:
TMPDIR=/usr ./creat01
tst_tmpdir.c:155: BROK: tst_tmpdir: mkdtemp(/usr/creF5DhMS) failed: EACCES


May be we should remove the strict check in tst_res.c? Otherwise we 
don't see the actual error.

diff --git a/lib/tst_res.c b/lib/tst_res.c
index b388d0d..dd27c54 100644
--- a/lib/tst_res.c
+++ b/lib/tst_res.c
@@ -547,11 +547,6 @@ void tst_brkm_(const char *file, const int lineno, 
int ttype,
         EXPAND_VAR_ARGS(tmesg, arg_fmt, USERMESG);

         if (tst_test) {
-               if (func) {
-                       tst_brk_(file, lineno, TBROK,
-                                "Non-NULL cleanup in newlib!");
- }
-
                 tst_brk_(file, lineno, ttype, "%s", tmesg);
         } else {
                 tst_brk__(file, lineno, ttype, func, "%s", tmesg);


Thanks,
Alexey

[LTP] new ltp lib doesn't print actual error

[Jan Stancek]

----- Original Message -----
> From: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> To: ltp@lists.linux.it
> Sent: Wednesday, 15 June, 2016 7:08:11 PM
> Subject: [LTP] new ltp lib doesn't print actual error
> 
> Hi,
> 
> When running a test with the new LTP lib, noticed that if some error
> occurs in a library (tst_tmpdir.c), it'll print the same message:
> 
> TMPDIR=/usr ./creat01
> tst_tmpdir.c:155: BROK: Non-NULL cleanup in newlib!
> 
> but should be:
> TMPDIR=/usr ./creat01
> tst_tmpdir.c:155: BROK: tst_tmpdir: mkdtemp(/usr/creF5DhMS) failed: EACCES
> 
> 
> May be we should remove the strict check in tst_res.c? Otherwise we
> don't see the actual error.

Or turn it into warning, so that we get both failures. tst_tmpdir() needs
a fix to not pass cleanup function for newlib testcases.

diff --git a/lib/tst_res.c b/lib/tst_res.c
index b388d0d2e516..ab9759b9263b 100644
--- a/lib/tst_res.c
+++ b/lib/tst_res.c
@@ -548,8 +548,8 @@ void tst_brkm_(const char *file, const int lineno, int ttype,

        if (tst_test) {
                if (func) {
-                       tst_brk_(file, lineno, TBROK,
-                                "Non-NULL cleanup in newlib!");
+                       tst_resm(TWARN, "Non-NULL cleanup in newlib! %s:%d",
+                               file, lineno);
                }

                tst_brk_(file, lineno, ttype, "%s", tmesg);

[LTP] new ltp lib doesn't print actual error

[Alexey Kodanev]

Hi,
On 06/15/2016 08:28 PM, Jan Stancek wrote:
>
>
>
> ----- Original Message -----
>> From: "Alexey Kodanev" <alexey.kodanev@oracle.com>
>> To: ltp@lists.linux.it
>> Sent: Wednesday, 15 June, 2016 7:08:11 PM
>> Subject: [LTP] new ltp lib doesn't print actual error
>>
>> Hi,
>>
>> When running a test with the new LTP lib, noticed that if some error
>> occurs in a library (tst_tmpdir.c), it'll print the same message:
>>
>> TMPDIR=/usr ./creat01
>> tst_tmpdir.c:155: BROK: Non-NULL cleanup in newlib!
>>
>> but should be:
>> TMPDIR=/usr ./creat01
>> tst_tmpdir.c:155: BROK: tst_tmpdir: mkdtemp(/usr/creF5DhMS) failed: EACCES
>>
>>
>> May be we should remove the strict check in tst_res.c? Otherwise we
>> don't see the actual error.
> Or turn it into warning, so that we get both failures. tst_tmpdir() needs
> a fix to not pass cleanup function for newlib testcases.
>
> diff --git a/lib/tst_res.c b/lib/tst_res.c
> index b388d0d2e516..ab9759b9263b 100644
> --- a/lib/tst_res.c
> +++ b/lib/tst_res.c
> @@ -548,8 +548,8 @@ void tst_brkm_(const char *file, const int lineno, int ttype,
>
>          if (tst_test) {
>                  if (func) {
> -                       tst_brk_(file, lineno, TBROK,
> -                                "Non-NULL cleanup in newlib!");
> +                       tst_resm(TWARN, "Non-NULL cleanup in newlib! %s:%d",
> +                               file, lineno);
>                  }
>
>                  tst_brk_(file, lineno, ttype, "%s", tmesg);

OK, we can do that, I just thought why we need to print it as it's not 
related to the test...

Best regards,
Alexey

[LTP] new ltp lib doesn't print actual error

[Cyril Hrubis]

Hi!
> When running a test with the new LTP lib, noticed that if some error 
> occurs in a library (tst_tmpdir.c), it'll print the same message:
> 
> TMPDIR=/usr ./creat01
> tst_tmpdir.c:155: BROK: Non-NULL cleanup in newlib!
> 
> but should be:
> TMPDIR=/usr ./creat01
> tst_tmpdir.c:155: BROK: tst_tmpdir: mkdtemp(/usr/creF5DhMS) failed: EACCES
> 
> 
> May be we should remove the strict check in tst_res.c? Otherwise we 
> don't see the actual error.

Passing a cleanup callback from newlib testcase is always a mistake
because it's never executed. I would prefer keeping the check in the
library.

It does not matter in case of tmpdir_cleanup() since that function only
prints a warning message. What about removing the tmpdir_cleanup()
cleanup instead? It does not seem to be useful to me anyway.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] new ltp lib doesn't print actual error

[Alexey Kodanev]

Hi,
On 06/20/2016 03:17 PM, Cyril Hrubis wrote:
> Hi!
>> When running a test with the new LTP lib, noticed that if some error
>> occurs in a library (tst_tmpdir.c), it'll print the same message:
>>
>> TMPDIR=/usr ./creat01
>> tst_tmpdir.c:155: BROK: Non-NULL cleanup in newlib!
>>
>> but should be:
>> TMPDIR=/usr ./creat01
>> tst_tmpdir.c:155: BROK: tst_tmpdir: mkdtemp(/usr/creF5DhMS) failed: EACCES
>>
>>
>> May be we should remove the strict check in tst_res.c? Otherwise we
>> don't see the actual error.
> Passing a cleanup callback from newlib testcase is always a mistake
> because it's never executed. I would prefer keeping the check in the
> library.
>
> It does not matter in case of tmpdir_cleanup() since that function only
> prints a warning message. What about removing the tmpdir_cleanup()
> cleanup instead? It does not seem to be useful to me anyway.

Agree, I've checked both old/new lib, and now it looks good. Applied patch.

Thanks,
Alexey