Re: [PATCH V5 06/25] oslib: qemu_clr_cloexec

[Steven Sistare <steven.sistare@oracle.com>]

On 7/8/2021 9:58 AM, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 7, 2021 at 9:33 PM Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>> wrote:
> 
>     Define qemu_clr_cloexec, analogous to qemu_set_cloexec.
> 
>     Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com <mailto:dgilbert@redhat.com>>
>     Signed-off-by: Steve Sistare <steven.sistare@oracle.com <mailto:steven.sistare@oracle.com>>
>     ---
>      include/qemu/osdep.h | 1 +
>      util/oslib-posix.c   | 9 +++++++++
>      util/oslib-win32.c   | 4 ++++
>      3 files changed, 14 insertions(+)
> 
>     diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>     index c91a78b..3d6a6ca 100644
>     --- a/include/qemu/osdep.h
>     +++ b/include/qemu/osdep.h
>     @@ -637,6 +637,7 @@ static inline void qemu_timersub(const struct timeval *val1,
>      #endif
> 
>      void qemu_set_cloexec(int fd);
>     +void qemu_clr_cloexec(int fd);
> 
> 
> I wish we would have a single function to set or unset, tbh. (as _clr_ isn't as readable to me)

I would rather not replace the existing qemu_set_cloexec calls, but I will expand clr to clear.

>      /* Starting on QEMU 2.5, qemu_hw_version() returns "2.5+" by default
>       * instead of QEMU_VERSION, so setting hw_version on MachineClass
>     diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>     index e8bdb02..97577f1 100644
>     --- a/util/oslib-posix.c
>     +++ b/util/oslib-posix.c
>     @@ -309,6 +309,15 @@ void qemu_set_cloexec(int fd)
>          assert(f != -1);
>      }
> 
>     +void qemu_clr_cloexec(int fd)
>     +{
>     +    int f;
>     +    f = fcntl(fd, F_GETFD);
>     +    assert(f != -1);
>     +    f = fcntl(fd, F_SETFD, f & ~FD_CLOEXEC);
>     +    assert(f != -1);
>     +}
> 
> 
> (asserting() may not be very judicious for calls that we intend to make during running time, but that's the way it is so far)

yep.

- Steve

>     +
>      /*
>       * Creates a pipe with FD_CLOEXEC set on both file descriptors
>       */
>     diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>     index af559ef..46e94d9 100644
>     --- a/util/oslib-win32.c
>     +++ b/util/oslib-win32.c
>     @@ -265,6 +265,10 @@ void qemu_set_cloexec(int fd)
>      {
>      }
> 
>     +void qemu_clr_cloexec(int fd)
>     +{
>     +}
>     +
>      /* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
>      #define _W32_FT_OFFSET (116444736000000000ULL)
> 
>     -- 
>     1.8.3.1
> 
> 
> 
> 
> -- 
> Marc-André Lureau