From mboxrd@z Thu Jan  1 00:00:00 1970
From: Cyrill Gorcunov <gorcunov@openvz.org>
Subject: Re: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
Date: Sat, 4 Feb 2012 22:02:19 +0400
Message-ID: <20120204180219.GD14818@moon>
References: <20120204163204.GB15888@moon>
 <alpine.LFD.2.02.1202041953230.1809@tux.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Sasha Levin <levinsasha928@gmail.com>,
	Asias He <asias.hejun@gmail.com>, Ingo Molnar <mingo@elte.hu>,
	KVM-ML <kvm@vger.kernel.org>
To: Pekka Enberg <penberg@kernel.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-bk0-f46.google.com ([209.85.214.46]:54673 "EHLO
	mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752663Ab2BDSCY (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sat, 4 Feb 2012 13:02:24 -0500
Received: by bkcjm19 with SMTP id jm19so3820676bkc.19
        for <kvm@vger.kernel.org>; Sat, 04 Feb 2012 10:02:22 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <alpine.LFD.2.02.1202041953230.1809@tux.localdomain>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sat, Feb 04, 2012 at 07:54:38PM +0200, Pekka Enberg wrote:
> On Sat, 4 Feb 2012, Cyrill Gorcunov wrote:
> >Index: linux-2.6.git/tools/kvm/kvm.c
> >===================================================================
> >--- linux-2.6.git.orig/tools/kvm/kvm.c
> >+++ linux-2.6.git/tools/kvm/kvm.c
> >@@ -123,10 +123,12 @@ static int kvm__check_extensions(struct
> >static struct kvm *kvm__new(void)
> >{
> >	struct kvm *kvm = calloc(1, sizeof(*kvm));
> >-
> >	if (!kvm)
> >		return ERR_PTR(-ENOMEM);
> >
> >+	kvm->sys_fd = -1;
> >+	kvm->vm_fd = -1;
> >+
> >	return kvm;
> >}
> >
> >@@ -394,9 +396,12 @@ struct kvm *kvm__init(const char *kvm_de
> >	kvm_ipc__start(kvm__create_socket(kvm));
> >	kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
> >	return kvm;
> >+
> >cleanup:
> >-	close(kvm->vm_fd);
> >-	close(kvm->sys_fd);
> >+	if (kvm->vm_fd >= 0)
> >+		close(kvm->vm_fd);
> >+	if (kvm->sys_fd >= 0)
> >+		close(kvm->sys_fd);
> 
> No, please don't do this! Use specific error handling labels instead.
> 

Strictly speaking, kvm__init need more serious rewrite together with
kvm__arch_init/kvm_ipc__start/kvm_ipc__register_handler ret. vals tests,
i'll do this a bit late.

	Cyrill