On 11/01/2016 14:05, Daniel P. Berrange wrote:
> The /dev/null file handle was leaked in a couple of places.
> There is also the possibility that both readfd and writefd
> point to the same /dev/null file handle, so care must be
> taken not to close the same file handle twice.
>
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> io/channel-command.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/io/channel-command.c b/io/channel-command.c
> index a220fe8..a9c67aa 100644
> --- a/io/channel-command.c
> +++ b/io/channel-command.c
> @@ -66,7 +66,7 @@ qio_channel_command_new_spawn(const char *const argv[],
>
> if (stdinnull || stdoutnull) {
> devnull = open("/dev/null", O_RDWR);
> - if (!devnull) {
> + if (devnull < 0) {
> error_setg_errno(errp, errno,
> "Unable to open /dev/null");
> goto error;
> @@ -98,6 +98,9 @@ qio_channel_command_new_spawn(const char *const argv[],
> close(stdoutfd[0]);
> close(stdoutfd[1]);
> }
> + if (devnull != -1) {
> + close(devnull);
> + }
>
> execv(argv[0], (char * const *)argv);
> _exit(1);
> @@ -117,6 +120,9 @@ qio_channel_command_new_spawn(const char *const argv[],
> return ioc;
>
> error:
> + if (devnull != -1) {
> + close(devnull);
> + }
> if (stdinfd[0] != -1) {
> close(stdinfd[0]);
> }
> @@ -202,12 +208,12 @@ static void qio_channel_command_finalize(Object *obj)
> QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj);
> if (ioc->readfd != -1) {
> close(ioc->readfd);
> - ioc->readfd = -1;
> }
> - if (ioc->writefd != -1) {
> + if (ioc->writefd != -1 &&
> + ioc->writefd != ioc->readfd) {
> close(ioc->writefd);
> - ioc->writefd = -1;
> }
> + ioc->writefd = ioc->readfd = -1;
> if (ioc->pid > 0) {
> #ifndef WIN32
> qio_channel_command_abort(ioc, NULL);
> @@ -299,12 +305,16 @@ static int qio_channel_command_close(QIOChannel *ioc,
> /* We close FDs before killing, because that
> * gives a better chance of clean shutdown
> */
> - if (close(cioc->writefd) < 0) {
> + if (cioc->readfd != -1 &&
> + close(cioc->readfd) < 0) {
> rv = -1;
> }
> - if (close(cioc->readfd) < 0) {
> + if (cioc->writefd != -1 &&
> + cioc->writefd != cioc->readfd &&
> + close(cioc->writefd) < 0) {
> rv = -1;
> }
> + cioc->writefd = cioc->readfd = -1;
> #ifndef WIN32
> if (qio_channel_command_abort(cioc, errp) < 0) {
> return -1;
>
Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
