linux-kernel - Re: [Bcache v13 11/16] bcache: Core btree code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 10 May 2012 14:48:49 -0700
From:	Kent Overstreet <koverstreet@...gle.com>
To:	Joe Perches <joe@...ches.com>
Cc:	linux-bcache@...r.kernel.org, linux-kernel@...r.kernel.org,
	dm-devel@...hat.com, tejun@...gle.com, agk@...hat.com
Subject: Re: [Bcache v13 11/16] bcache: Core btree code

I don't feel strongly one way or the other about it, but I do think
it's more just a matter of taste - the if (0) is ugly, I'll certainly
grant you that but IMO it makes the weird control flow harder to miss,
and the indentation more or less matches the control flow.

But I haven't come up with a way of writing that I actually like, I
dislike them all almost equally.

On Thu, May 10, 2012 at 11:49 AM, Joe Perches <joe@...ches.com> wrote:
> On Wed, 2012-05-09 at 23:10 -0400, Kent Overstreet wrote:
>> Signed-off-by: Kent Overstreet <koverstreet@...gle.com>
> []
>> +
>> +void btree_read_done(struct closure *cl)
>> +{
> []
>> +     if (b->written < btree_blocks(b))
>> +             bset_init_next(b);
>> +
>> +     if (0) {
>> +err:         set_btree_node_io_error(b);
>> +             cache_set_error(b->c, "%s at bucket %lu, block %zu, %u keys",
>> +                             err, PTR_BUCKET_NR(b->c, &b->key, 0),
>> +                             index(i, b), i->keys);
>> +     }
>
> Hi Kent
>
> trivia:  This if (0) is an exceedingly ugly style.
>
> I'd much prefer:
>
>        if (foo)
>                bar();
>
>        goto exit;
>
> err:
>        set_btree_node_io_error(b);
>        cache_set_error(b->c, "%s at bucket %lu, block %zu, %u keys",
>                        err, PTR_BUCKET_NR(b->c, &b->key, 0),
>                        index(i, b), i->keys);
>
> exit:
>        etc...
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/