From: Florian Fainelli <florian@openwrt•org>
To: "David Laight" <David.Laight@aculab•com>
Cc: davem@davemloft•net, netdev@vger•kernel.org
Subject: Re: [PATCH 3/3] dsa: fix freeing of sparse port allocation
Date: Mon, 25 Mar 2013 20:55:33 +0100 [thread overview]
Message-ID: <201303252055.33840.florian@openwrt.org> (raw)
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B71B3@saturn3.aculab.com>
Le lundi 25 mars 2013 17:03:23, David Laight a écrit :
> > If we have defined a sparse port allocation which is non-contiguous and
> > contains gaps, the code freeing port_names will just stop when it
> > encouters a first NULL port_names, which is not right, we should iterate
> > over all possible number of ports (DSA_MAX_PORTS) until we are done.
>
> ...
>
> > port_index = 0;
> >
> > - while (pd->chip[i].port_names &&
> > - pd->chip[i].port_names[++port_index])
> > - kfree(pd->chip[i].port_names[port_index]);
> > + while (port_index < DSA_MAX_PORTS) {
> > + if (pd->chip[i].port_names[port_index])
> > + kfree(pd->chip[i].port_names[port_index]);
> > + port_index++;
> > + }
>
> A couple of 'odd' differences:
>
> The old code checked pd->chip[i].port_names (as if it
> might be a pointer) that is absent from the new version.
> (If it is separately allocated it is leaked).
>
> The new code tests and frees pd->chip[i].port_names[0]
> whereas the old code ignored the 0th entry.
The old code was wrong, it was off by one for the first array index, and would
stop whenever it encountered the first NULL port_names[index] so we would not
skip other these and free the possibly non-NULL next one.
I think the current code is now correct, but thanks for the review!
--
Florian
next prev parent reply other threads:[~2013-03-25 19:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-25 15:03 [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings Florian Fainelli
2013-03-25 15:03 ` [PATCH 1/3] dsa: fix device tree binding documentation typo on #address-cells Florian Fainelli
2013-03-25 15:03 ` [PATCH 2/3] dsa: factor freeing of dsa_platform_data Florian Fainelli
2013-03-25 15:03 ` [PATCH 3/3] dsa: fix freeing of sparse port allocation Florian Fainelli
2013-03-25 16:03 ` David Laight
2013-03-25 19:55 ` Florian Fainelli [this message]
2013-03-25 16:23 ` [PATCH 0/3 net-next] dsa: bugfixing and typos after Device Tree bindings David Miller
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=201303252055.33840.florian@openwrt.org \
--to=florian@openwrt$(echo .)org \
--cc=David.Laight@aculab$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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