From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52973) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNiYh-0003rY-GA for qemu-devel@nongnu.org; Tue, 08 Nov 2011 05:05:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RNiYc-0003sG-6k for qemu-devel@nongnu.org; Tue, 08 Nov 2011 05:05:35 -0500 Date: Tue, 8 Nov 2011 10:06:02 +0000 From: "Daniel P. Berrange" Message-ID: <20111108100601.GB15005@redhat.com> References: <1320746152-31620-1-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1320746152-31620-1-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH] ui/vnc: Convert sasl.mechlist to g_malloc() & friends Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org On Tue, Nov 08, 2011 at 10:55:52AM +0100, Markus Armbruster wrote: > Fixes protocol_client_auth_sasl_mechname() not to crash when malloc() > fails. Spotted by Coverity. > > Signed-off-by: Markus Armbruster > --- > ui/vnc-auth-sasl.c | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c > index 23b1bf5..a88973b 100644 > --- a/ui/vnc-auth-sasl.c > +++ b/ui/vnc-auth-sasl.c > @@ -35,7 +35,7 @@ void vnc_sasl_client_cleanup(VncState *vs) > vs->sasl.encodedLength = vs->sasl.encodedOffset = 0; > vs->sasl.encoded = NULL; > g_free(vs->sasl.username); > - free(vs->sasl.mechlist); > + g_free(vs->sasl.mechlist); > vs->sasl.username = vs->sasl.mechlist = NULL; > sasl_dispose(&vs->sasl.conn); > vs->sasl.conn = NULL; > @@ -430,7 +430,7 @@ static int protocol_client_auth_sasl_start_len(VncState *vs, uint8_t *data, size > > static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, size_t len) > { > - char *mechname = malloc(len + 1); > + char *mechname = g_malloc(len + 1); > if (!mechname) { > VNC_DEBUG("Out of memory reading mechname\n"); > vnc_client_error(vs); You can delete the if (!mechname) block now you have g_malloc The reason for the crash on OOM is here, but the diff context doesn't show it: Notice the missing 'return -1' statement following vnc_client_error(vs); char *mechname = malloc(len + 1); if (!mechname) { VNC_DEBUG("Out of memory reading mechname\n"); vnc_client_error(vs); } strncpy(mechname, (char*)data, len); mechname[len] = '\0'; > @@ -460,7 +460,7 @@ static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, size_ > } > } > > - free(vs->sasl.mechlist); > + g_free(vs->sasl.mechlist); > vs->sasl.mechlist = mechname; > > VNC_DEBUG("Validated mechname '%s'\n", mechname); > @@ -469,7 +469,7 @@ static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, size_ > > fail: > vnc_client_error(vs); > - free(mechname); > + g_free(mechname); > return -1; > } > > @@ -608,7 +608,7 @@ void start_auth_sasl(VncState *vs) > } > VNC_DEBUG("Available mechanisms for client: '%s'\n", mechlist); > > - if (!(vs->sasl.mechlist = strdup(mechlist))) { > + if (!(vs->sasl.mechlist = g_strdup(mechlist))) { > VNC_DEBUG("Out of memory"); > sasl_dispose(&vs->sasl.conn); > vs->sasl.conn = NULL; Again, you can delete the conditional here with g_strdup 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 :|