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?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
