On Mon, Jun 17, 2013 at 09:15:39PM +0200, Lorenz Haspel wrote:
> Fixes checkpatch error:
> There were assignments in if conditions, so I extracted them.
>
> Signed-off-by: Lorenz Haspel <lor...@badgers.com>
> Signed-off-by: Michael Banken <michael.ban...@mathe.stud.uni-erlangen.de>
> ---
> v2: removed some buggy extra lines and fixed white space issues
Gar.... This isn't right either. Now it has *too many* blank
lines. It's only between declarations and code that I was
complaining about. You've added them between assignments and error
checks.
> @@ -1224,7 +1237,9 @@ static int wdt_pulse(bpctl_dev_t *pbpctl_dev)
> return -1;
> #endif
> if (pbpctl_dev->bp_10g9) {
> - if (!(pbpctl_dev_c = get_status_port_fn(pbpctl_dev)))
> + pbpctl_dev_c = get_status_port_fn(pbpctl_dev);
> +
This blank line is harmful.
> + if (!pbpctl_dev_c)
> return -1;
> }
>
> @@ -1742,9 +1757,9 @@ 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;
> + bpctl_dev_t *pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
>
This blank line is required.
So what you have here is fine, but if you wanted you could re-write
this like:
{
bpctl_dev_t *pbpctl_dev_b;
pbpctl_dev_b = get_status_port_fn(pbpctl_dev);
if (!pbpctl_dev_b)
return -1;
Generally, you shouldn't put anything complicated in the initializer
statement. People don't read that code as thouroughly and
initializers are sometimes a source of bugs. But what you have here
is also perfectly acceptable.
> - if (!(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);
rergards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
