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

From: Dan Carpenter
Date: Mon Jun 17 2013 - 13:24:18 EST


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/