Re: [PATCH v2] util: fix fd leak in qemu_write_pidfile()

[Peter Maydell <peter.maydell@linaro.org>]

On Mon, 10 May 2021 at 15:15, Jie Wang <wangjie88@huawei.com> wrote:
>
> if execute qemu_open success, have no branch to free the fd,
> so unlink it inadvance, let it free by process exit.
>
> Signed-off-by: Jie Wang <wangjie88@huawei.com>
> ---
>  util/oslib-posix.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 36820fec16..fa881f2ee8 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -131,6 +131,7 @@ bool qemu_write_pidfile(const char *path, Error **errp)
>              error_setg_errno(errp, errno, "Cannot open pid file");
>              return false;
>          }
> +        unlink(path);
>
>          if (fstat(fd, &b) < 0) {
>              error_setg_errno(errp, errno, "Cannot stat file");

This code change doesn't match the commit message -- the commit
message says it's trying to free a filedescriptor, but the code
change is unlinking a file.

Unlinking the file is definitely wrong, because the purpose of the
pidfile is to comminucate the QEMU PID to other processes -- if we
delete the file then those other processes can't find it. (The file
gets deleted when QEMU exits -- see qemu_unlink_pidfile() and the code
that calls it.)

thanks
-- PMM