On Sun, Mar 08, 2015 at 05:54:07PM +0100, Matteo Semenzato wrote: > From: Matteo Semenzato <mattew8898@xxxxxxxxx> > > Return -ENOMEM if allocating brd->flipbuf fails. > > Signed-off-by: Matteo Semenzato <mattew8898@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxxx > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel