From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37223) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ab4it-0001Y4-1s for qemu-devel@nongnu.org; Wed, 02 Mar 2016 06:13:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ab4iq-0005h9-AU for qemu-devel@nongnu.org; Wed, 02 Mar 2016 06:13:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41853) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ab4iq-0005h2-4F for qemu-devel@nongnu.org; Wed, 02 Mar 2016 06:13:40 -0500 Date: Wed, 2 Mar 2016 11:13:36 +0000 From: "Daniel P. Berrange" Message-ID: <20160302111336.GC16292@redhat.com> References: <921079541.7300112.1456305734547.JavaMail.zimbra@ocs.online.net> <20160302110702.GC11268@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160302110702.GC11268@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] [PATCH] vl.c: fix memleaks with g_strdup+strtok Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Quentin PEREZ , qemu-devel@nongnu.org On Wed, Mar 02, 2016 at 11:07:02AM +0000, Stefan Hajnoczi wrote: > On Wed, Feb 24, 2016 at 10:22:14AM +0100, Quentin PEREZ wrote: > > diff --git a/vl.c b/vl.c > > index b87e292..9f6593a 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -1362,16 +1362,19 @@ static int add_semihosting_arg(void *opaque, > > static inline void semihosting_arg_fallback(const char *file, const char *cmd) > > { > > char *cmd_token; > > + char *dup_cmd; > > > > /* argv[0] */ > > add_semihosting_arg(&semihosting, "arg", file, NULL); > > > > /* split -append and initialize argv[1..n] */ > > - cmd_token = strtok(g_strdup(cmd), " "); > > + dup_cmd = g_strdup(cmd); > > + cmd_token = strtok(dup_cmd, " "); > > while (cmd_token) { > > add_semihosting_arg(&semihosting, "arg", cmd_token, NULL); > > cmd_token = strtok(NULL, " "); > > } > > + g_free(dup_cmd); > > add_semihosting_arg() stashes the cmd_token pointer. semihosting.argv[] > points to freed memory if you add g_free(dup_cmd). > > I suggest leaving the code as-is since the lifetime of the semihosting > global variable spans the entire run-time of the QEMU process. It's not > pretty but the leak is harmless. > > If you really want to fix it you may need to add a semihosting_cleanup() > function to free strings. If fixing it I'd probably suggest using g_strsplit() instead of strdup+strtok too, as it accepts a const string & returns allocated strings which is a nicer contract that strtok which modifies its arg Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|