From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36804)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1UvuqL-0005sM-V1
	for qemu-devel@nongnu.org; Sun, 07 Jul 2013 15:41:59 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1UvuqK-0001U6-Uz
	for qemu-devel@nongnu.org; Sun, 07 Jul 2013 15:41:57 -0400
Received: from v220110690675601.yourvserver.net ([78.47.199.172]:50624)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1UvuqK-0001Tp-Ob
	for qemu-devel@nongnu.org; Sun, 07 Jul 2013 15:41:56 -0400
Message-ID: <51D9C480.8020307@weilnetz.de>
Date: Sun, 07 Jul 2013 21:41:52 +0200
From: Stefan Weil <sw@weilnetz.de>
MIME-Version: 1.0
References: <1371376960-18192-1-git-send-email-sw@weilnetz.de>
	<51D33999.6000806@weilnetz.de> <87y59i9tq2.fsf@linux.vnet.ibm.com>
In-Reply-To: <87y59i9tq2.fsf@linux.vnet.ibm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] fsdev: Fix potential memory leak
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, qemu-devel <qemu-devel@nongnu.org>

Am 07.07.2013 19:15, schrieb Aneesh Kumar K.V:
> Stefan Weil <sw@weilnetz.de> writes:
>
>> Am 16.06.2013 12:02, schrieb Stefan Weil:
>>> This leak was reported by cppcheck.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>>  fsdev/qemu-fsdev.c |    2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
>>> index 6eaf36d..ccfec13 100644
>>> --- a/fsdev/qemu-fsdev.c
>>> +++ b/fsdev/qemu-fsdev.c
>>> @@ -76,6 +76,8 @@ int qemu_fsdev_add(QemuOpts *opts)
>>>  
>>>      if (fsle->fse.ops->parse_opts) {
>>>          if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) {
>>> +            g_free(fsle->fse.fsdev_id);
>>> +            g_free(fsle);
>>>              return -1;
>>>          }
>>>      }
>> Ping?
> Both the error path result in Qemu terminating right ? Do we really care
> about free in those case ?
>
> -aneesh


Yes, we do, because it simplifies code reviews if there are
as few as possible warnings from static code analysers.

It is also not immediately obvious that QEMU terminates
in the code which was patched here. Reviewers have
to grep and look in vl.c, too, to see this.

Therefore I'd appreciate if this and similar code could be
fixed (with low priority here).

Regards
Stefan W.