linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ivo van Doorn <ivdoorn@gmail.com>
To: Gertjan van Wingerde <gwingerde@kpnplanet.nl>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	linux-wireless@vger.kernel.org,
	rt2400-devel@lists.sourceforge.net
Subject: Re: [PATCH] rt2x00: Remove duplicate deinitialization
Date: Sat, 21 Jun 2008 12:04:19 +0200	[thread overview]
Message-ID: <200806211204.21129.IvDoorn@gmail.com> (raw)
In-Reply-To: <485C25CA.6000607@kpnplanet.nl>

Hi

> > @@ -509,16 +498,11 @@ static int rt2x00queue_alloc_rxskbs(struct rt2x00_dev *rt2x00dev,
> >  	for (i = 0; i < queue->limit; i++) {
> >  		skb = rt2x00queue_alloc_rxskb(rt2x00dev, &queue->entries[i]);
> >  		if (!skb)
> > -			goto exit;
> > +			return -ENOMEM;
> >  		queue->entries[i].skb = skb;
> >  	}
> >  
> >  	return 0;
> > -
> > -exit:
> > -	rt2x00queue_free_skbs(rt2x00dev, queue);
> > -
> > -	return -ENOMEM;
> >  }
> >  
> >  int rt2x00queue_initialize(struct rt2x00_dev *rt2x00dev)
> >   
> 
> 
> I find this last hunk of the patch a bit dubious. IMHO a function should 
> never rely on letting his caller do the right thing and clean up for it, 
> in case of failures; it should always clean up after itself. It is true 
> that in this case the caller will also attempt to clean up these 
> structures, but that doesn't mean that any potential future users of 
> such a function should be bothered with doing the same thing.

True but rt2x00queue_alloc_rxskbs and rt2x00queue_free_rxskbs are only
called once. And there isn't any reason why they should be called anywhere
else other then initialize() and uninitialize().

And this approach is the same as other allocation/free paths like in rt2x00pci/usb
and rt2x00lib.

Ivo

      reply	other threads:[~2008-06-21  9:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-20 20:10 [PATCH] rt2x00: Remove duplicate deinitialization Ivo van Doorn
2008-06-20 21:48 ` Gertjan van Wingerde
2008-06-21 10:04   ` Ivo van Doorn [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200806211204.21129.IvDoorn@gmail.com \
    --to=ivdoorn@gmail.com \
    --cc=gwingerde@kpnplanet.nl \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=rt2400-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).