Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753912Ab0HKQXc (ORCPT ); Wed, 11 Aug 2010 12:23:32 -0400 Received: from mgw2.diku.dk ([130.225.96.92]:47263 "EHLO mgw2.diku.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752537Ab0HKQXb (ORCPT ); Wed, 11 Aug 2010 12:23:31 -0400 Date: Wed, 11 Aug 2010 18:23:28 +0200 (CEST) From: Julia Lawall To: Patrick Gefre Cc: linux-ia64@vger.kernel.org, linux-mips@linux-mips.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure In-Reply-To: <4C62C8F3.3090405@sgi.com> Message-ID: References: <4C62C8F3.3090405@sgi.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1880 Lines: 48 > > I believe this code also leaks earlier instances of port, which are only > > referenced by card_ptr, which is freed in the error handling code at the > > end of the function. A lot of operations are done on port on each > > iteration, however, so I'm not sure whether it is good enough to just free > > them. Perhaps there is some way to call ioc3uart_remove? > > > > Yes you are right, there should be something like this for out4: > > out4: > for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) { > port = card_ptr->ic_port[phys_port].icp_port; > if (port) { > pci_free_consistent(port->ip_idd->pdev, > TOTAL_RING_BUF_SIZE, > (void *)port->ip_cpu_ringbuf, > port->ip_dma_ringbuf); > kfree(port); > } > } > kfree(card_ptr); > return ret; Actually, pci_alloc_consistent is only called when phys_port is 0. In the subsequent cases, the ip_dma_ringbuf field is just initialized to the previous value. So it could be: out4: for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) { port = card_ptr->ic_port[phys_port].icp_port; if (port) { if (phys_port == 0) pci_free_consistent(port->ip_idd->pdev, TOTAL_RING_BUF_SIZE, (void *)port->ip_cpu_ringbuf, port->ip_dma_ringbuf); kfree(port); } } kfree(card_ptr); return ret; julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/