<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Viresh,</p>
<p>Please ignore this reply.</p>
<p>Thanks,</p>
<p>Xiao Yang<br>
</p>
<div class="moz-cite-prefix">On 5/4/20 4:30 PM, 杨晓 wrote:<br>
</div>
<blockquote type="cite"
cite="mid:43140aa8.315c.171dece8540.Coremail.ice_yangxiao@163.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<br>
----- Original Message -----
<br>
From: "Viresh Kumar" <a class="moz-txt-link-rfc2396E" href="mailto:viresh.kumar@linaro.org"><viresh.kumar@linaro.org></a>
<br>
To: "Xiao Yang" <a class="moz-txt-link-rfc2396E" href="mailto:yangx.jy@cn.fujitsu.com"><yangx.jy@cn.fujitsu.com></a>
<br>
Cc: <a class="moz-txt-link-abbreviated" href="mailto:ltp@lists.linux.it">ltp@lists.linux.it</a>
<br>
Sent: Mon, 4 May 2020 10:39:37 +0530
<br>
Subject: Re: [LTP] [PATCH 1/2] syscalls/pidfd_open01.c: Add check for close-on-exec flag
<br>
<br>
On 30-04-20, 16:57, Xiao Yang wrote:<br>
> pidfd_open(2) will set close-on-exec flag on the file descriptor as it<br>
> manpage states, so check close-on-exec flag by fcntl(2).<br>
> <br>
> Also avoid compiler warning by replacing (long) TST_RET with (int) pidfd:<br>
> ------------------------------------------------------<br>
> In file included from pidfd_open01.c:9:<br>
> pidfd_open01.c: In function ‘run’:<br>
> ../../../../include/tst_test.h:76:41: warning: format ‘%i’ expects argument of type ‘int’, but argument 5 has type ‘long int’ [-Wformat=]<br>
> 76 | tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\<br>
> | ^~~~~~~~~<br>
> ../../../../include/tst_safe_macros.h:224:5: note: in expansion of macro ‘tst_brk’<br>
> 224 | tst_brk(TBROK | TERRNO, \<br>
> | ^~~~~~~<br>
> pidfd_open01.c:20:9: note: in expansion of macro ‘SAFE_FCNTL’<br>
> 20 | flag = SAFE_FCNTL(TST_RET, F_GETFD);<br>
<br>
This log isn't useful as the warning started coming after your change<br>
only and not before.
<div><br>
</div>
<div>Hi，</div>
<div><br>
</div>
<div>Right，just add a hint why I use pidfd instead so I want to
keep it. Of course，I will <span style="background-color:
transparent;">say that avoid compiler warning from my change
in v2 patch.</span></div>
<div><br>
</div>
<div>Thanks，</div>
<div>Xiao Yang</div>
<div><br>
> ------------------------------------------------------<br>
> <br>
> Signed-off-by: Xiao Yang <a class="moz-txt-link-rfc2396E" href="mailto:yangx.jy@cn.fujitsu.com"><yangx.jy@cn.fujitsu.com></a><br>
> ---<br>
> .../kernel/syscalls/pidfd_open/pidfd_open01.c | 18 ++++++++++++++----<br>
> 1 file changed, 14 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c<br>
> index 93bb86687..293e93b63 100644<br>
> --- a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c<br>
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c<br>
> @@ -6,17 +6,27 @@<br>
> * Basic pidfd_open() test, fetches the PID of the current process and tries to<br>
> * get its file descriptor.<br>
> */<br>
> +<br>
> +#include <sys/types.h><br>
> +#include <unistd.h><br>
> +#include <fcntl.h><br>
> #include "tst_test.h"<br>
> #include "lapi/pidfd_open.h"<br>
> <br>
> static void run(void)<br>
> {<br>
> - TEST(pidfd_open(getpid(), 0));<br>
> + int pidfd = 0, flag = 0;<br>
<br>
None of these need to be initialized.<br>
<br>
> +<br>
> + pidfd = pidfd_open(getpid(), 0);<br>
> + if (pidfd == -1)<br>
> +
tst_brk(TFAIL | TERRNO, "pidfd_open(getpid(), 0) failed");<br>
<br>
This could have been written as:<br>
TEST(pidfd = pidfd_open(getpid(), 0));<br>
<br>
> +<br>
> + flag = SAFE_FCNTL(pidfd, F_GETFD);<br>
> <br>
> - if (TST_RET == -1)<br>
> -
tst_brk(TFAIL | TTERRNO, "pidfd_open(getpid(), 0) failed");<br>
> + SAFE_CLOSE(pidfd);<br>
> <br>
> - SAFE_CLOSE(TST_RET);<br>
> + if (!(flag & FD_CLOEXEC))<br>
> +
tst_brk(TFAIL, "pidfd_open(getpid(), 0) didn't set close-on-exec flag");<br>
> <br>
> tst_res(TPASS, "pidfd_open(getpid(), 0) passed");<br>
> }<br>
> -- <br>
> 2.21.0<br>
> <br>
> <br>
<br>
-- <br>
viresh<br>
<br>
-- <br>
Mailing list info: <a class="moz-txt-link-freetext" href="https://lists.linux.it/listinfo/ltp">https://lists.linux.it/listinfo/ltp</a><br>
</div>
<br>
<br>
<span title="neteasefooter">
<p> </p>
</span><br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>