[PATCH]Please review the patch

Randy Dunlap rdunlap at xenotime.net
Fri Jun 22 11:03:41 CDT 2007


On Fri, 22 Jun 2007 12:42:08 +0530 pradeep singh wrote:

> On 6/21/07, Matt Mackall <mpm at selenic.com> wrote:
> > On Thu, Jun 21, 2007 at 04:28:42PM +0530, pradeep singh wrote:
> [snip]
> > >
> > > This patch fixes this by checking for NULL in the test condition for, for loop.
> >
> > You probably want to instead check it before entering the loop:
> 
> Thanks Matt ,fixed that yesterday but could not get formatting correct
> with gmail.
> Is this correct now?
> 
> Signed-off-by: Pradeep Singh <pradeep.rautela at gmail.com>
> ---
>  drivers/net/chelsio/cxgb2.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/chelsio/cxgb2.c b/drivers/net/chelsio/cxgb2.c
> index 231ce43..006c634 100644
> --- a/drivers/net/chelsio/cxgb2.c
> +++ b/drivers/net/chelsio/cxgb2.c
> @@ -1022,6 +1022,11 @@ static int __devinit init_one(struct pci_dev *pdev,
>  	mmio_start = pci_resource_start(pdev, 0);
>  	mmio_len = pci_resource_len(pdev, 0);
>  	bi = t1_get_board_info(ent->driver_data);
> +	

Above line ends with a tab  :(

> +	if (!bi) {
> +                CH_ERR("%s: Board info array index out of
> range\n",pci_name(pdev));
> +                goto out_disable_pdev;
> +        }

Above 3 lines use spaces to indent -- should use tabs.
Also, the CH_ERR() line was broken/split somewhere:  then "range" line
shouldn't be a separate line, and the diff does not count it as a
separate line.

> 
>  	for (i = 0; i < bi->port_number; ++i) {
>  		struct net_device *netdev;

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***


More information about the Kernel-mentors mailing list