[PATCH 2/3] staging: et131x: Refactor nic_rx_pkts() to remove indenting

Dan Carpenter dan.carpenter at oracle.com
Sat Oct 27 12:44:54 UTC 2012


On Sat, Oct 27, 2012 at 11:03:16AM +0100, mark.einon at gmail.com 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.

regards,
dan carpenter




More information about the devel mailing list