Pavel Roskin <proski@xxxxxxx> :
> On Sat, 2005-05-14 at 19:39 +0200, Francois Romieu wrote:
> > > + drop:
> > > + stats->rx_errors++;
> > > + stats->rx_dropped++;
> > > +}
> >
> > -> leak (skb).
>
> Indeed. Thank you! Please apply this on top of the original patches:
>
> Signed-off-by: Pavel Roskin <proski@xxxxxxx>
>
> --- orinoco.c
> +++ orinoco.c
> @@ -1180,7 +1180,7 @@ static void orinoco_rx_monitor(struct ne
> u16 fc;
> int err;
> int len;
> - struct sk_buff *skb;
> + struct sk_buff *skb = NULL;
> struct orinoco_private *priv = netdev_priv(dev);
> struct net_device_stats *stats = &priv->stats;
> hermes_t *hw = &priv->hw;
> @@ -1268,6 +1268,8 @@ static void orinoco_rx_monitor(struct ne
> drop:
> stats->rx_errors++;
> stats->rx_dropped++;
> + if (skb)
> + dev_kfree_skb_irq(skb);
> }
>
> static void __orinoco_ev_rx(struct net_device *dev, hermes_t *hw)
>
>
I am not fond of unneeded/hidden state variable. What about:
+ /* sanity check the length */
+ if (datalen > IEEE80211_DATA_LEN + 12) {
+ printk(KERN_DEBUG "%s: oversized monitor frame, "
+ "data length = %d\n", dev->name, datalen);
+ err = -EIO;
+ goto drop;
^^^^^^^^^^ -> let's replace by 'goto update_stats;'
And turn:
+ drop:
+ stats->rx_errors++;
+ stats->rx_dropped++;
into:
+ drop:
+ dev_kfree_skb_irq(skb);
+ update_stats:
+ stats->rx_errors++;
+ stats->rx_dropped++;
This way 'goto drop' really drops and the code does not issue a 'goto drop'
when it actually want to update the stats.
--
Ueimor
|