Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions

[Dan Carpenter]

This will need to be redone because there were some buggy extra
lines added toward the end of the patch.

On Mon, Jun 17, 2013 at 06:26:23PM +0200, Lorenz Haspel wrote:
> @@ -1743,8 +1750,8 @@ static void write_data_port_int(bpctl_dev_t *pbpctl_dev,
>  static int write_data_int(bpctl_dev_t *pbpctl_dev, unsigned char value)
>  {
>  	bpctl_dev_t *pbpctl_dev_b = NULL;
> -

This blank line is needed.  (Put a blank line between declarations
and code).

> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +	if (!pbpctl_dev_b)
>  		return -1;
>  	atomic_set(&pbpctl_dev->wdt_busy, 1);
>  	write_data_port_int(pbpctl_dev, value & 0x3);
> @@ -1965,7 +1972,8 @@ int tpl_hw_on(bpctl_dev_t *pbpctl_dev)
>  	int ret = 0, ctrl = 0;
>  	bpctl_dev_t *pbpctl_dev_b = NULL;
>  
> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +	if (!pbpctl_dev_b)
>  		return BP_NOT_CAP;
>  
>  	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {

> @@ -1991,8 +1999,8 @@ int tpl_hw_off(bpctl_dev_t *pbpctl_dev)
>  {
>  	int ret = 0, ctrl = 0;
>  	bpctl_dev_t *pbpctl_dev_b = NULL;
> -

Same for this blank line and all the following instances.

> -	if (!(pbpctl_dev_b = get_status_port_fn(pbpctl_dev)))
> +	pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +	if (!pbpctl_dev_b)
>  		return BP_NOT_CAP;
>  	if (pbpctl_dev->bp_caps_ex & TPL2_CAP_EX) {
>  		cmnd_on(pbpctl_dev);

> @@ -4461,11 +4472,13 @@ int set_bypass_pwoff_fn(bpctl_dev_t *pbpctl_dev, int bypass_mode)
>  
>  	if (!(pbpctl_dev->bp_caps & BP_PWOFF_CTL_CAP))
>  		return BP_NOT_CAP;
> -	if ((ret = cmnd_on(pbpctl_dev)) < 0)
> +	ret = cmnd_on(pbpctl_dev);
> +	if (ret < 0)
>  		return ret;
>  	if (bypass_mode)
>  		ret = bypass_state_pwroff(pbpctl_dev);
> -	else
> +	ret = cmnd_on(pbpctl_dev);
> +	if (ret < 0)
>  		ret = normal_state_pwroff(pbpctl_dev);

These added lines do not belong.  The patch will need to be redone
to fix this bug.

>  	cmnd_off(pbpctl_dev);
>  	return ret;

> @@ -4867,10 +4884,12 @@ int set_tx_fn(bpctl_dev_t *pbpctl_dev, int tx_state)
>  	    (pbpctl_dev->bp_caps & SW_CTL_CAP)) {
>  		if ((pbpctl_dev->bp_tpl_flag))
>  			return BP_NOT_CAP;
> -	} else if ((pbpctl_dev_b = get_master_port_fn(pbpctl_dev))) {
> -		if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
> -		    (pbpctl_dev_b->bp_tpl_flag))
> -			return BP_NOT_CAP;
> +	} else {
> +		pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
> +		if (pbpctl_dev_b)
> +			if ((pbpctl_dev_b->bp_caps & TPL_CAP) &&
> +			    (pbpctl_dev_b->bp_tpl_flag))
> +				return BP_NOT_CAP;

Please put curly brace {} around multi-line indents.  Even though
they are not needed for semantic reasons they make the code more
readable.

>  	}
>  	return set_tx(pbpctl_dev, tx_state);
>  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/