* [PATCH] generic/486: print correct errno
@ 2018-05-16 8:00 Xiao Yang
2018-05-16 15:27 ` Darrick J. Wong
0 siblings, 1 reply; 7+ messages in thread
From: Xiao Yang @ 2018-05-16 8:00 UTC (permalink / raw)
To: fstests; +Cc: libc-alpha-help, Xiao Yang
On glibc v2.12 or v2.17, running generic/486 with XFS got wrong
EINVAL instead of ENODATA:
----------------------------------------------------------------
QA output created by 486
No data available
error=22 at line 63
----------------------------------------------------------------
We want to save the correct errno and print it after calling perror().
It seems that only calling perror() didn't change the errno, but calling
perror() and redirecting the output of perror() to a file set errno to
EINVAL.
For example, running the special test.c to reproduce the issue:
------------------------------------------------------------
#include <errno.h>
#include <unistd.h>
#include <stdio.h>
int main(void)
{
close(-1);
printf("errno %d before\n", errno);
perror("");
printf("errno %d after\n", errno);
}
------------------------------------------------------------
# gcc -o test test.c
# ./test
# ./test 2>log
fdopen() called by perror() in glibc seems to set errno to EINVAL
in some cases(e.g. the access mode of stderr is O_WRONLY). I am not
sure whether this is a perror bug in glibc or not.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
src/attr_replace_test.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
index 23adc07..e9a08cc 100644
--- a/src/attr_replace_test.c
+++ b/src/attr_replace_test.c
@@ -10,8 +10,9 @@
#include <sys/xattr.h>
#include <sys/stat.h>
-#define die() do { perror(""); \
-fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
+#define die() do { int real_errno = errno; \
+perror(""); \
+fprintf(stderr, "error=%d at line %d\n", real_errno, __LINE__); \
exit(1); } while (0)
#define fail(...) do { \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] generic/486: print correct errno
2018-05-16 8:00 [PATCH] generic/486: print correct errno Xiao Yang
@ 2018-05-16 15:27 ` Darrick J. Wong
2018-05-17 1:36 ` [PATCH] generic/486: Get rid of the redundant error=%d printing Xiao Yang
0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-16 15:27 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests, libc-alpha-help
On Wed, May 16, 2018 at 04:00:04PM +0800, Xiao Yang wrote:
> On glibc v2.12 or v2.17, running generic/486 with XFS got wrong
> EINVAL instead of ENODATA:
> ----------------------------------------------------------------
> QA output created by 486
> No data available
> error=22 at line 63
This is a regression test for a kernel patch; the XATTR_REPLACE is
supposed to succeed if you've added the fix. If it throws any kind of
error then the kernel is misbehaving, but it doesn't really matter which
error we get.
However, we could just get rid of the redundant error=%d printing,
seeing as we just perror'd anyway.
--D
> ----------------------------------------------------------------
>
> We want to save the correct errno and print it after calling perror().
>
> It seems that only calling perror() didn't change the errno, but calling
> perror() and redirecting the output of perror() to a file set errno to
> EINVAL.
>
> For example, running the special test.c to reproduce the issue:
> ------------------------------------------------------------
> #include <errno.h>
> #include <unistd.h>
> #include <stdio.h>
>
> int main(void)
> {
> close(-1);
> printf("errno %d before\n", errno);
> perror("");
> printf("errno %d after\n", errno);
> }
> ------------------------------------------------------------
> # gcc -o test test.c
> # ./test
> # ./test 2>log
>
> fdopen() called by perror() in glibc seems to set errno to EINVAL
> in some cases(e.g. the access mode of stderr is O_WRONLY). I am not
> sure whether this is a perror bug in glibc or not.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> src/attr_replace_test.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 23adc07..e9a08cc 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -10,8 +10,9 @@
> #include <sys/xattr.h>
> #include <sys/stat.h>
>
> -#define die() do { perror(""); \
> -fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
> +#define die() do { int real_errno = errno; \
> +perror(""); \
> +fprintf(stderr, "error=%d at line %d\n", real_errno, __LINE__); \
> exit(1); } while (0)
>
> #define fail(...) do { \
> --
> 1.8.3.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] generic/486: Get rid of the redundant error=%d printing
2018-05-16 15:27 ` Darrick J. Wong
@ 2018-05-17 1:36 ` Xiao Yang
2018-05-17 3:16 ` Eryu Guan
0 siblings, 1 reply; 7+ messages in thread
From: Xiao Yang @ 2018-05-17 1:36 UTC (permalink / raw)
To: darrick.wong; +Cc: fstests, Xiao Yang
1) Without the fix, perror() can indicate the actual error(ENODATA).
2) After calling perror() and redirecting the output of perror()
to a file, errno seems to be set to EINVAL unexpectedly.
See the following mail for detailed info:
https://www.spinics.net/lists/fstests/msg09675.html
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
src/attr_replace_test.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
index 23adc07..33fa74d 100644
--- a/src/attr_replace_test.c
+++ b/src/attr_replace_test.c
@@ -11,7 +11,6 @@
#include <sys/stat.h>
#define die() do { perror(""); \
-fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
exit(1); } while (0)
#define fail(...) do { \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] generic/486: Get rid of the redundant error=%d printing
2018-05-17 1:36 ` [PATCH] generic/486: Get rid of the redundant error=%d printing Xiao Yang
@ 2018-05-17 3:16 ` Eryu Guan
2018-05-17 3:25 ` Xiao Yang
2018-05-17 3:32 ` [PATCH v3] " Xiao Yang
0 siblings, 2 replies; 7+ messages in thread
From: Eryu Guan @ 2018-05-17 3:16 UTC (permalink / raw)
To: Xiao Yang; +Cc: darrick.wong, fstests
On Thu, May 17, 2018 at 09:36:10AM +0800, Xiao Yang wrote:
> 1) Without the fix, perror() can indicate the actual error(ENODATA).
> 2) After calling perror() and redirecting the output of perror()
> to a file, errno seems to be set to EINVAL unexpectedly.
> See the following mail for detailed info:
> https://www.spinics.net/lists/fstests/msg09675.html
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> src/attr_replace_test.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 23adc07..33fa74d 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -11,7 +11,6 @@
> #include <sys/stat.h>
>
> #define die() do { perror(""); \
> -fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
We can remove the error number in fprintf, but I think the line number
info is still useful (at least it helped me when I was reviewing the
test).
Thanks,
Eryu
> exit(1); } while (0)
>
> #define fail(...) do { \
> --
> 1.8.3.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] generic/486: Get rid of the redundant error=%d printing
2018-05-17 3:16 ` Eryu Guan
@ 2018-05-17 3:25 ` Xiao Yang
2018-05-17 3:32 ` [PATCH v3] " Xiao Yang
1 sibling, 0 replies; 7+ messages in thread
From: Xiao Yang @ 2018-05-17 3:25 UTC (permalink / raw)
To: Eryu Guan; +Cc: darrick.wong, fstests
On 2018/05/17 11:16, Eryu Guan wrote:
> On Thu, May 17, 2018 at 09:36:10AM +0800, Xiao Yang wrote:
>> 1) Without the fix, perror() can indicate the actual error(ENODATA).
>> 2) After calling perror() and redirecting the output of perror()
>> to a file, errno seems to be set to EINVAL unexpectedly.
>> See the following mail for detailed info:
>> https://www.spinics.net/lists/fstests/msg09675.html
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> src/attr_replace_test.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
>> index 23adc07..33fa74d 100644
>> --- a/src/attr_replace_test.c
>> +++ b/src/attr_replace_test.c
>> @@ -11,7 +11,6 @@
>> #include<sys/stat.h>
>>
>> #define die() do { perror(""); \
>> -fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
> We can remove the error number in fprintf, but I think the line number
> info is still useful (at least it helped me when I was reviewing the
> test).
Hi Eryu,
OK, we just remove errno in fprintf() as below:
fprintf(stderr, "error at line %d\n", __LINE__);
Thanks,
Xiao Yang
> Thanks,
> Eryu
>
>> exit(1); } while (0)
>>
>> #define fail(...) do { \
>> --
>> 1.8.3.1
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3] generic/486: Get rid of the redundant error=%d printing
2018-05-17 3:16 ` Eryu Guan
2018-05-17 3:25 ` Xiao Yang
@ 2018-05-17 3:32 ` Xiao Yang
2018-05-17 14:55 ` Darrick J. Wong
1 sibling, 1 reply; 7+ messages in thread
From: Xiao Yang @ 2018-05-17 3:32 UTC (permalink / raw)
To: guaneryu; +Cc: darrick.wong, fstests, Xiao Yang
1) Without the fix, perror() can indicate the actual error(ENODATA).
2) After calling perror() and redirecting the output of perror()
to a file, errno seems to be set to EINVAL unexpectedly.
See the following mail for detailed info:
https://www.spinics.net/lists/fstests/msg09675.html
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
src/attr_replace_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
index 23adc07..0720bfd 100644
--- a/src/attr_replace_test.c
+++ b/src/attr_replace_test.c
@@ -11,7 +11,7 @@
#include <sys/stat.h>
#define die() do { perror(""); \
-fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
+fprintf(stderr, "error at line %d\n", __LINE__); \
exit(1); } while (0)
#define fail(...) do { \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3] generic/486: Get rid of the redundant error=%d printing
2018-05-17 3:32 ` [PATCH v3] " Xiao Yang
@ 2018-05-17 14:55 ` Darrick J. Wong
0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2018-05-17 14:55 UTC (permalink / raw)
To: Xiao Yang; +Cc: guaneryu, fstests
On Thu, May 17, 2018 at 11:32:27AM +0800, Xiao Yang wrote:
> 1) Without the fix, perror() can indicate the actual error(ENODATA).
> 2) After calling perror() and redirecting the output of perror()
> to a file, errno seems to be set to EINVAL unexpectedly.
> See the following mail for detailed info:
> https://www.spinics.net/lists/fstests/msg09675.html
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> src/attr_replace_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index 23adc07..0720bfd 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -11,7 +11,7 @@
> #include <sys/stat.h>
>
> #define die() do { perror(""); \
> -fprintf(stderr, "error=%d at line %d\n", errno, __LINE__); \
> +fprintf(stderr, "error at line %d\n", __LINE__); \
> exit(1); } while (0)
>
> #define fail(...) do { \
> --
> 1.8.3.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-17 14:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-16 8:00 [PATCH] generic/486: print correct errno Xiao Yang
2018-05-16 15:27 ` Darrick J. Wong
2018-05-17 1:36 ` [PATCH] generic/486: Get rid of the redundant error=%d printing Xiao Yang
2018-05-17 3:16 ` Eryu Guan
2018-05-17 3:25 ` Xiao Yang
2018-05-17 3:32 ` [PATCH v3] " Xiao Yang
2018-05-17 14:55 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox