On 27 October 2012 13:44, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> On Sat, Oct 27, 2012 at 11:03:16AM +0100, mark.einon@xxxxxxxxx wrote:
>> @@ -2748,7 +2747,7 @@ static struct rfd *nic_rx_pkts(struct et131x_adapter *adapter)
>> struct rx_ring *rx_local = &adapter->rx_ring;
>> struct rx_status_block *status;
>> struct pkt_stat_desc *psr;
>> - struct rfd *rfd;
>> + struct rfd *rfd = NULL;
> ^^^^^^^^^^
> Not needed.
>
>> u32 i;
>> u8 *buf;
>> unsigned long flags;
>> @@ -2758,6 +2757,7 @@ static struct rfd *nic_rx_pkts(struct et131x_adapter *adapter)
>> u32 len;
>> u32 word0;
>> u32 word1;
>> + struct sk_buff *skb = NULL;
> ^^^^^^^^^^
> Not needed. Don't introduce unneeded assignments. It defeats the
> error checking in GCC and has leads to NULL dereference bugs.
>
>> - if (len) {
>> - /* Determine if this is a multicast packet coming in */
>> - if ((word0 & ALCATEL_MULTICAST_PKT) &&
>> - !(word0 & ALCATEL_BROADCAST_PKT)) {
>> - /* Promiscuous mode and Multicast mode are
>> - * not mutually exclusive as was first
>> - * thought. I guess Promiscuous is just
>> - * considered a super-set of the other
>> - * filters. Generally filter is 0x2b when in
>> - * promiscuous mode.
>> - */
>> - if ((adapter->packet_filter &
>> - ET131X_PACKET_TYPE_MULTICAST)
>> - && !(adapter->packet_filter &
>> - ET131X_PACKET_TYPE_PROMISCUOUS)
>> - && !(adapter->packet_filter &
>> + if (len == 0)
>> + goto out;
>> +
>
> Should this be:
>
> if (len == 0) {
> rfd->len = 0;
> goto out;
> }
>
> That would match the original code better, but I'm not sure if it
> is necessary.
Hi Dan,
Thanks for pointing out these issues. I'll assume a separate patch
will be fine to fix them, which I'll send shortly.
cheers,
Mark
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
