From: Greg Kurz <groug@kaod.org>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: qemu-devel@nongnu.org, xen-devel@lists.xen.org,
anthony.perard@citrix.com, aneesh.kumar@linux.vnet.ibm.com,
Eric Blake <eblake@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl
Date: Tue, 9 May 2017 22:54:55 +0200 [thread overview]
Message-ID: <20170509225455.694ef4dc@bahia> (raw)
In-Reply-To: <1494356693-13190-3-git-send-email-sstabellini@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]
On Tue, 9 May 2017 12:04:53 -0700
Stefano Stabellini <sstabellini@kernel.org> wrote:
> Use the common utility function, which contains checks on return values,
... and first calls F_GETFD as recommended by POSIX.1-2001.
http://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html
"The arg values to F_GETFD, F_SETFD, F_GETFL, and F_SETFL all represent
flag values to allow for future growth. Applications using these functions
should do a read-modify-write operation on them, rather than assuming that
only the values defined by this volume of IEEE Std 1003.1-2001 are valid.
It is a common error to forget this, particularly in the case of F_SETFD."
> instead of manually calling fcntl.
>
> CID: 1374831
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: anthony.perard@citrix.com
> CC: groug@kaod.org
> CC: aneesh.kumar@linux.vnet.ibm.com
> CC: Eric Blake <eblake@redhat.com>
> ---
> hw/9pfs/xen-9p-backend.c | 2 +-
> hw/xen/xen_backend.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
Anyway,
Reviewed-by: Greg Kurz <groug@kaod.org>
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index a1fdede..5df97c9 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -380,7 +380,7 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
> if (xen_9pdev->rings[i].evtchndev == NULL) {
> goto out;
> }
> - fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev), F_SETFD,
> FD_CLOEXEC);
> + qemu_set_cloexec(xenevtchn_fd(xen_9pdev->rings[i].evtchndev));
> xen_9pdev->rings[i].local_port = xenevtchn_bind_interdomain
> (xen_9pdev->rings[i].evtchndev,
> xendev->dom,
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index c85f163..2cac47d 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -147,7 +147,7 @@ static struct XenDevice *xen_be_get_xendev(const char
> *type, int dom, int dev, qdev_unplug(DEVICE(xendev), NULL);
> return NULL;
> }
> - fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> + qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
>
> if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
> xendev->gnttabdev = xengnttab_open(NULL, 0);
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kurz <groug@kaod.org>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: anthony.perard@citrix.com, aneesh.kumar@linux.vnet.ibm.com,
Eric Blake <eblake@redhat.com>,
qemu-devel@nongnu.org, xen-devel@lists.xen.org
Subject: Re: [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl
Date: Tue, 9 May 2017 22:54:55 +0200 [thread overview]
Message-ID: <20170509225455.694ef4dc@bahia> (raw)
In-Reply-To: <1494356693-13190-3-git-send-email-sstabellini@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 2420 bytes --]
On Tue, 9 May 2017 12:04:53 -0700
Stefano Stabellini <sstabellini@kernel.org> wrote:
> Use the common utility function, which contains checks on return values,
... and first calls F_GETFD as recommended by POSIX.1-2001.
http://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html
"The arg values to F_GETFD, F_SETFD, F_GETFL, and F_SETFL all represent
flag values to allow for future growth. Applications using these functions
should do a read-modify-write operation on them, rather than assuming that
only the values defined by this volume of IEEE Std 1003.1-2001 are valid.
It is a common error to forget this, particularly in the case of F_SETFD."
> instead of manually calling fcntl.
>
> CID: 1374831
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> CC: anthony.perard@citrix.com
> CC: groug@kaod.org
> CC: aneesh.kumar@linux.vnet.ibm.com
> CC: Eric Blake <eblake@redhat.com>
> ---
> hw/9pfs/xen-9p-backend.c | 2 +-
> hw/xen/xen_backend.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
Anyway,
Reviewed-by: Greg Kurz <groug@kaod.org>
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index a1fdede..5df97c9 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -380,7 +380,7 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
> if (xen_9pdev->rings[i].evtchndev == NULL) {
> goto out;
> }
> - fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev), F_SETFD,
> FD_CLOEXEC);
> + qemu_set_cloexec(xenevtchn_fd(xen_9pdev->rings[i].evtchndev));
> xen_9pdev->rings[i].local_port = xenevtchn_bind_interdomain
> (xen_9pdev->rings[i].evtchndev,
> xendev->dom,
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index c85f163..2cac47d 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -147,7 +147,7 @@ static struct XenDevice *xen_be_get_xendev(const char
> *type, int dom, int dev, qdev_unplug(DEVICE(xendev), NULL);
> return NULL;
> }
> - fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> + qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
>
> if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
> xendev->gnttabdev = xengnttab_open(NULL, 0);
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-05-09 20:55 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-09 19:04 [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity Stefano Stabellini
2017-05-09 19:04 ` Stefano Stabellini
2017-05-09 19:04 ` [Qemu-devel] [PATCH v2 2/3] Check the return value of fcntl in qemu_set_cloexec Stefano Stabellini
2017-05-09 19:04 ` Stefano Stabellini
2017-05-09 19:20 ` [Qemu-devel] " Eric Blake
2017-05-09 19:20 ` Eric Blake
2017-05-09 20:31 ` [Qemu-devel] " Greg Kurz
2017-05-09 20:31 ` Greg Kurz
2017-05-11 7:55 ` [Qemu-devel] " Paolo Bonzini
2017-05-11 7:55 ` Paolo Bonzini
2017-05-16 18:53 ` [Qemu-devel] " Stefano Stabellini
2017-05-16 18:53 ` Stefano Stabellini
2017-05-09 19:04 ` [Qemu-devel] [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl Stefano Stabellini
2017-05-09 19:04 ` Stefano Stabellini
2017-05-09 19:21 ` [Qemu-devel] " Eric Blake
2017-05-09 19:21 ` Eric Blake
2017-05-09 20:54 ` Greg Kurz [this message]
2017-05-09 20:54 ` Greg Kurz
2017-05-09 19:20 ` [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity Eric Blake
2017-05-09 19:20 ` Eric Blake
2017-05-09 19:28 ` Eric Blake
2017-05-09 19:28 ` Eric Blake
2017-05-09 19:47 ` Stefano Stabellini
2017-05-09 19:47 ` Stefano Stabellini
2017-05-09 20:29 ` Greg Kurz
2017-05-09 20:29 ` Greg Kurz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170509225455.694ef4dc@bahia \
--to=groug@kaod.org \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=anthony.perard@citrix.com \
--cc=eblake@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.