From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754392AbbCILYe (ORCPT ); Mon, 9 Mar 2015 07:24:34 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:22444 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754364AbbCILYa (ORCPT ); Mon, 9 Mar 2015 07:24:30 -0400 Date: Mon, 9 Mar 2015 14:24:12 +0300 From: Dan Carpenter To: Matteo Semenzato Cc: gregkh@linuxfoundation.org, lidza.louina@gmail.com, markh@compro.net, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Staging: dgnc: check kzalloc result Message-ID: <20150309112412.GC10964@mwanda> References: <1425833647-21686-1-git-send-email-mattew8898@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1425833647-21686-1-git-send-email-mattew8898@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: ucsinet21.oracle.com [156.151.31.93] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Mar 08, 2015 at 05:54:07PM +0100, Matteo Semenzato wrote: > From: Matteo Semenzato > > Return -ENOMEM if allocating brd->flipbuf fails. > > Signed-off-by: Matteo Semenzato > --- > drivers/staging/dgnc/dgnc_driver.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c > index f177d3a..711af02 100644 > --- a/drivers/staging/dgnc/dgnc_driver.c > +++ b/drivers/staging/dgnc/dgnc_driver.c > @@ -622,6 +622,10 @@ static int dgnc_found_board(struct pci_dev *pdev, int id) > * context, and there are no locks held. > */ > brd->flipbuf = kzalloc(MYFLIPLEN, GFP_KERNEL); > + if (!brd->flipbuf) { > + kfree(brd); > + return -ENOMEM; This leaves a freed pointer in "dgnc_Board[dgnc_NumBoards]" which is sort of ugly. It doesn't free "brd->msgbuf_head" which is a bug. Please read my essay on error hanling: https://plus.google.com/106378716002406849458 What we want is : ret = do_one(); if (ret) return ret; ret = do_two(); if (ret) goto undo_one; ret = do_three(); if (ret) goto undo_two; return 0; err_undo_two: undo_two(); err_undo_one: undo_one(); return ret; Basically, a list of commands and a mirror list of undos. This is standard kernel style and the error handling basically writes itself automatically. regards, dan carpenter > + } > > wake_up_interruptible(&brd->state_wait); > > -- > 2.3.1 > > _______________________________________________ > devel mailing list > devel@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel