* superfluous " else if ()"
@ 2014-04-26 18:57 Toralf Förster
2014-05-09 16:49 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Toralf Förster @ 2014-04-26 18:57 UTC (permalink / raw)
To: Chris Mason; +Cc: linux-btrfs
/me wonders if this
if (ret >= 0) {
/* Add an item for the type for the first time */
eb = path->nodes[0];
slot = path->slots[0];
offset = btrfs_item_ptr_offset(eb, slot);
} else if (ret == -EEXIST) {
/*
* An item with that type already exists.
* Extend the item and store the new subid at the end.
*/
btrfs_extend_item(uuid_root, path, sizeof(subid_le));
eb = path->nodes[0];
slot = path->slots[0];
offset = btrfs_item_ptr_offset(eb, slot);
offset += btrfs_item_size_nr(eb, slot) - sizeof(subid_le);
} else if (ret < 0) { <-----------------
btrfs_warn(uuid_root->fs_info, "insert uuid item failed %d "
"(0x%016llx, 0x%016llx) type %u!",
ret, (unsigned long long)key.objectid,
(unsigned long long)key.offset, type);
goto out;
}
shouldn't be condensed into just "} else {" ?
--
Toralf
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: superfluous " else if ()"
2014-04-26 18:57 superfluous " else if ()" Toralf Förster
@ 2014-05-09 16:49 ` David Sterba
2014-05-09 17:12 ` Toralf Förster
0 siblings, 1 reply; 4+ messages in thread
From: David Sterba @ 2014-05-09 16:49 UTC (permalink / raw)
To: Toralf Förster; +Cc: Chris Mason, linux-btrfs
On Sat, Apr 26, 2014 at 08:57:00PM +0200, Toralf Förster wrote:
> /me wonders if this
>
> if (ret >= 0) {
> /* Add an item for the type for the first time */
> eb = path->nodes[0];
> slot = path->slots[0];
> offset = btrfs_item_ptr_offset(eb, slot);
> } else if (ret == -EEXIST) {
> /*
> * An item with that type already exists.
> * Extend the item and store the new subid at the end.
> */
> btrfs_extend_item(uuid_root, path, sizeof(subid_le));
> eb = path->nodes[0];
> slot = path->slots[0];
> offset = btrfs_item_ptr_offset(eb, slot);
> offset += btrfs_item_size_nr(eb, slot) - sizeof(subid_le);
> } else if (ret < 0) { <-----------------
> btrfs_warn(uuid_root->fs_info, "insert uuid item failed %d "
> "(0x%016llx, 0x%016llx) type %u!",
> ret, (unsigned long long)key.objectid,
> (unsigned long long)key.offset, type);
> goto out;
> }
>
>
> shouldn't be condensed into just "} else {" ?
It's equivalent to the original code, but is easier to read/understand.
Do you have there other concerns besides readability?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: superfluous " else if ()"
2014-05-09 16:49 ` David Sterba
@ 2014-05-09 17:12 ` Toralf Förster
2014-05-12 14:15 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Toralf Förster @ 2014-05-09 17:12 UTC (permalink / raw)
To: dsterba, Chris Mason, linux-btrfs
On 05/09/2014 06:49 PM, David Sterba wrote:
> On Sat, Apr 26, 2014 at 08:57:00PM +0200, Toralf Förster wrote:
>> /me wonders if this
>>
>> if (ret >= 0) {
>> /* Add an item for the type for the first time */
>> eb = path->nodes[0];
>> slot = path->slots[0];
>> offset = btrfs_item_ptr_offset(eb, slot);
>> } else if (ret == -EEXIST) {
>> /*
>> * An item with that type already exists.
>> * Extend the item and store the new subid at the end.
>> */
>> btrfs_extend_item(uuid_root, path, sizeof(subid_le));
>> eb = path->nodes[0];
>> slot = path->slots[0];
>> offset = btrfs_item_ptr_offset(eb, slot);
>> offset += btrfs_item_size_nr(eb, slot) - sizeof(subid_le);
>> } else if (ret < 0) { <-----------------
>> btrfs_warn(uuid_root->fs_info, "insert uuid item failed %d "
>> "(0x%016llx, 0x%016llx) type %u!",
>> ret, (unsigned long long)key.objectid,
>> (unsigned long long)key.offset, type);
>> goto out;
>> }
>>
>>
>> shouldn't be condensed into just "} else {" ?
>
> It's equivalent to the original code, but is easier to read/understand.
> Do you have there other concerns besides readability?
>
Oh no, if it is intended and b/c it is not in a runtime critical path - no.
But what came into my mind, wouldn't the following avoid such question in future ? :
} else {
/* ret < 0 */
--
Toralf
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-12 14:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-26 18:57 superfluous " else if ()" Toralf Förster
2014-05-09 16:49 ` David Sterba
2014-05-09 17:12 ` Toralf Förster
2014-05-12 14:15 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).