From: Greg Kurz <groug@kaod.org>
To: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: <qemu-devel@nongnu.org>, Alex Chen <alex.chen@huawei.com>,
<qemu-trivial@nongnu.org>, <zhang.zhanghailiang@huawei.com>
Subject: Re: [PATCH] virtfs-proxy-helper: Fix a resource leak in main()
Date: Fri, 27 Nov 2020 10:10:26 +0100 [thread overview]
Message-ID: <20201127101026.2ca28dc9@bahia.lan> (raw)
In-Reply-To: <13615133.E8RLdmFOkC@silver>
On Thu, 26 Nov 2020 19:44:24 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
[...]
> > The only justification that'd deserve to be in the changelog of
> > such a patch is something like "because this is good practice
> > to rollback in case code moves to another function than main()".
>
> Well, the actual motivation was rather a pragmatic one: to shut up a
> sanitizer's false positive, which I can understand.
>
Yes, this should also be mentioned in the changelog.
> Another option would be using a global variable for the fd instead of a
> temporary on stack. That should shut up the sanitizer as well and would not
> introduce change to the program flow.
>
Using the same sock variable for an fd that is either passed to us
or that we create is a very poor programming choice actually... :(
So if the motivation is just to make "Euler Robot" happy and this
can be addressed as you suggest, I personally prefer that rather
than piling up fixes on broken code.
> I leave that up to Greg to decide whether or not to handle this. I'm
> Switzerland on this one.
>
This won't go into QEMU 5.2 anyway since we only merge fixes for
critical bugs or regressions at this point. No hurry to decide
anything now :)
> Best regards,
> Christian Schoenebeck
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kurz <groug@kaod.org>
To: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: Alex Chen <alex.chen@huawei.com>,
qemu-trivial@nongnu.org, qemu-devel@nongnu.org,
zhang.zhanghailiang@huawei.com
Subject: Re: [PATCH] virtfs-proxy-helper: Fix a resource leak in main()
Date: Fri, 27 Nov 2020 10:10:26 +0100 [thread overview]
Message-ID: <20201127101026.2ca28dc9@bahia.lan> (raw)
In-Reply-To: <13615133.E8RLdmFOkC@silver>
On Thu, 26 Nov 2020 19:44:24 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
[...]
> > The only justification that'd deserve to be in the changelog of
> > such a patch is something like "because this is good practice
> > to rollback in case code moves to another function than main()".
>
> Well, the actual motivation was rather a pragmatic one: to shut up a
> sanitizer's false positive, which I can understand.
>
Yes, this should also be mentioned in the changelog.
> Another option would be using a global variable for the fd instead of a
> temporary on stack. That should shut up the sanitizer as well and would not
> introduce change to the program flow.
>
Using the same sock variable for an fd that is either passed to us
or that we create is a very poor programming choice actually... :(
So if the motivation is just to make "Euler Robot" happy and this
can be addressed as you suggest, I personally prefer that rather
than piling up fixes on broken code.
> I leave that up to Greg to decide whether or not to handle this. I'm
> Switzerland on this one.
>
This won't go into QEMU 5.2 anyway since we only merge fixes for
critical bugs or regressions at this point. No hurry to decide
anything now :)
> Best regards,
> Christian Schoenebeck
>
>
next prev parent reply other threads:[~2020-11-27 9:10 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-26 10:16 [PATCH] virtfs-proxy-helper: Fix a resource leak in main() Alex Chen
2020-11-26 10:16 ` Alex Chen
2020-11-26 10:50 ` Li Qiang
2020-11-26 10:50 ` Li Qiang
2020-11-26 11:40 ` Alex Chen
2020-11-26 11:40 ` Alex Chen
2020-11-26 15:04 ` Li Qiang
2020-11-26 15:04 ` Li Qiang
2020-11-26 12:07 ` Greg Kurz
2020-11-26 12:07 ` Greg Kurz
2020-11-26 13:15 ` Alex Chen
2020-11-26 13:15 ` Alex Chen
2020-11-26 17:52 ` Christian Schoenebeck
2020-11-26 18:27 ` Greg Kurz
2020-11-26 18:27 ` Greg Kurz
2020-11-26 18:44 ` Christian Schoenebeck
2020-11-27 9:10 ` Greg Kurz [this message]
2020-11-27 9:10 ` 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=20201127101026.2ca28dc9@bahia.lan \
--to=groug@kaod.org \
--cc=alex.chen@huawei.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-trivial@nongnu.org \
--cc=qemu_oss@crudebyte.com \
--cc=zhang.zhanghailiang@huawei.com \
/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.