Messages in this thread | | | Date | Mon, 17 Jun 2013 20:22:46 +0300 | From | Dan Carpenter <> | Subject | Re: [PATCH 1/4] silicom: checkpatch: assignments in if conditions |
| |
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
| |