Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932124Ab1BPHuR (ORCPT ); Wed, 16 Feb 2011 02:50:17 -0500 Received: from isilmar-3.linta.de ([188.40.101.200]:39756 "EHLO linta.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757250Ab1BPHuK (ORCPT ); Wed, 16 Feb 2011 02:50:10 -0500 Date: Wed, 16 Feb 2011 08:21:18 +0100 From: Dominik Brodowski To: Julia Lawall Cc: Jiri Kosina , kernel-janitors@vger.kernel.org, David Sterba , linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/5] drivers/char/pcmcia/ipwireless/main.c: Convert release_resource to release_region/release_mem_region Message-ID: <20110216072118.GA27029@comet.dominikbrodowski.net> Mail-Followup-To: Dominik Brodowski , Julia Lawall , Jiri Kosina , kernel-janitors@vger.kernel.org, David Sterba , linux-kernel@vger.kernel.org References: <1297599132-7226-1-git-send-email-julia@diku.dk> <1297599132-7226-4-git-send-email-julia@diku.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1297599132-7226-4-git-send-email-julia@diku.dk> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6084 Lines: 175 Applied to the pcmcia treee, thanks. On Sun, Feb 13, 2011 at 01:12:10PM +0100, Julia Lawall wrote: > Request_region should be used with release_region, not release_resource. > > This patch contains a number of changes, related to calls to request_region, > request_mem_region, and the associated error handling code. > > 1. For the call to request_region, the variable io_resource storing the > result is dropped. The call to release_resource at the end of the function > is changed to a call to release_region with the first two arguments of > request_region as its arguments. The same call to release_region is also > added to release_ipwireless. > > 2. The first call to request_mem_region is now tested and ret is set to > -EBUSY if the the call has failed. This call was associated with the > initialization of ipw->attr_memory. But the error handling code was > testing ipw->common_memory. The definition of release_ipwireless also > suggests that this call should be associated with ipw->common_memory, not > ipw->attr_memory. > > 3. The second call to request_mem_region is now tested and ret is > set to -EBUSY if the the call has failed. > > 4. The various gotos to the error handling code is adjusted so that there > is no need for ifs. > > 5. Return the value stored in the ret variable rather than -1. > > The semantic match that finds this problem is as follows: > (http://coccinelle.lip6.fr/) > > // > @@ > expression x,E; > @@ > ( > *x = request_region(...) > | > *x = request_mem_region(...) > ) > ... when != release_region(x) > when != x = E > * release_resource(x); > // > > Signed-off-by: Julia Lawall > > --- > Compiled, not tested. > In release_ipwireless, should the added call to release_region be protected > by some if? > > drivers/char/pcmcia/ipwireless/main.c | 52 ++++++++++++++++++++-------------- > 1 file changed, 32 insertions(+), 20 deletions(-) > > diff --git a/drivers/char/pcmcia/ipwireless/main.c b/drivers/char/pcmcia/ipwireless/main.c > index 94b8eb4..444155a 100644 > --- a/drivers/char/pcmcia/ipwireless/main.c > +++ b/drivers/char/pcmcia/ipwireless/main.c > @@ -78,7 +78,6 @@ static void signalled_reboot_callback(void *callback_data) > static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data) > { > struct ipw_dev *ipw = priv_data; > - struct resource *io_resource; > int ret; > > p_dev->resource[0]->flags &= ~IO_DATA_PATH_WIDTH; > @@ -92,9 +91,12 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data) > if (ret) > return ret; > > - io_resource = request_region(p_dev->resource[0]->start, > - resource_size(p_dev->resource[0]), > - IPWIRELESS_PCCARD_NAME); > + if (!request_region(p_dev->resource[0]->start, > + resource_size(p_dev->resource[0]), > + IPWIRELESS_PCCARD_NAME)) { > + ret = -EBUSY; > + goto exit; > + } > > p_dev->resource[2]->flags |= > WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_CM | WIN_ENABLE; > @@ -105,22 +107,25 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data) > > ret = pcmcia_map_mem_page(p_dev, p_dev->resource[2], p_dev->card_addr); > if (ret != 0) > - goto exit2; > + goto exit1; > > ipw->is_v2_card = resource_size(p_dev->resource[2]) == 0x100; > > - ipw->attr_memory = ioremap(p_dev->resource[2]->start, > + ipw->common_memory = ioremap(p_dev->resource[2]->start, > resource_size(p_dev->resource[2])); > - request_mem_region(p_dev->resource[2]->start, > - resource_size(p_dev->resource[2]), > - IPWIRELESS_PCCARD_NAME); > + if (!request_mem_region(p_dev->resource[2]->start, > + resource_size(p_dev->resource[2]), > + IPWIRELESS_PCCARD_NAME)) { > + ret = -EBUSY; > + goto exit2; > + } > > p_dev->resource[3]->flags |= WIN_DATA_WIDTH_16 | WIN_MEMORY_TYPE_AM | > WIN_ENABLE; > p_dev->resource[3]->end = 0; /* this used to be 0x1000 */ > ret = pcmcia_request_window(p_dev, p_dev->resource[3], 0); > if (ret != 0) > - goto exit2; > + goto exit3; > > ret = pcmcia_map_mem_page(p_dev, p_dev->resource[3], 0); > if (ret != 0) > @@ -128,23 +133,28 @@ static int ipwireless_probe(struct pcmcia_device *p_dev, void *priv_data) > > ipw->attr_memory = ioremap(p_dev->resource[3]->start, > resource_size(p_dev->resource[3])); > - request_mem_region(p_dev->resource[3]->start, > - resource_size(p_dev->resource[3]), > - IPWIRELESS_PCCARD_NAME); > + if (!request_mem_region(p_dev->resource[3]->start, > + resource_size(p_dev->resource[3]), > + IPWIRELESS_PCCARD_NAME)) { > + ret = -EBUSY; > + goto exit4; > + } > > return 0; > > +exit4: > + iounmap(ipw->attr_memory); > exit3: > + release_mem_region(p_dev->resource[2]->start, > + resource_size(p_dev->resource[2])); > exit2: > - if (ipw->common_memory) { > - release_mem_region(p_dev->resource[2]->start, > - resource_size(p_dev->resource[2])); > - iounmap(ipw->common_memory); > - } > + iounmap(ipw->common_memory); > exit1: > - release_resource(io_resource); > + release_region(p_dev->resource[0]->start, > + resource_size(p_dev->resource[0])); > +exit: > pcmcia_disable_device(p_dev); > - return -1; > + return ret; > } > > static int config_ipwireless(struct ipw_dev *ipw) > @@ -219,6 +229,8 @@ exit: > > static void release_ipwireless(struct ipw_dev *ipw) > { > + release_region(ipw->link->resource[0]->start, > + resource_size(ipw->link->resource[0])); > if (ipw->common_memory) { > release_mem_region(ipw->link->resource[2]->start, > resource_size(ipw->link->resource[2])); > > -- > 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/ -- 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/