[haiku-bugs] Re: [Haiku] #13612: BTRFS: Adding write support

  • From: "hyche" <trac@xxxxxxxxxxxx>
  • Date: Sun, 23 Jul 2017 10:49:45 -0000

#13612: BTRFS: Adding write support
----------------------------------+----------------------------
   Reporter:  hyche               |      Owner:  korli
       Type:  enhancement         |     Status:  new
   Priority:  normal              |  Milestone:  Unscheduled
  Component:  File Systems/Btrfs  |    Version:  R1/Development
 Resolution:                      |   Keywords:  gsoc2017
 Blocked By:                      |   Blocking:
Has a Patch:  1                   |   Platform:  All
----------------------------------+----------------------------

Comment (by hyche):

 Replying to [comment:10 pulkomandy]:

In the first change (style fixes), in btrfs_read_attr_dir:

{{{

+       if (status != B_OK)
+               return status;
+
        if (status == B_ENTRY_NOT_FOUND) {
                *_num = 0;
                return B_OK;
-       } else if (status != B_OK)
-               return status;
+       }

}}}

The previous code would ignore the B_ENTRY_NOT_FOUND error and return
 B_OK instead. Now it will fail. Is that intentional?
 No, that was a mistake, I don't know why me at that time doing this,
 thanks for pointing out.


I remember I was the one suggesting BTreeNode and BTreePath, but reading
 this code again I > think using BTree::Node and BTree::Path may make more
 sense (as inner-classes or in a > namespace?)
 I moved it into BTree class, but not sure about code style, please check
 again.


Now we have a Logical() here that is not the same thing as the Logical()
 in
8ee5e1382f7cc0bf66bb317e84a7295e8c07b84d. What was wrong with
 LogicalRoot()?
 A tree has only one root, so logical address of the tree would be
 understood as LogicalRoot, that is my original thought, but anyway fix it.


I assume the goal of the "read" flag is to create the entry if it does
 not exist, for write >support. If that is the case, then createIfNotFound
 would be a more descriptive name than >"read".
 Yes, but it doesn't create inside the find function, it only retrieve the
 slot for later. Beside, I just let it there to separate write and read
 operations, I won't touch it until next week.


 Add more line breaks in commit message.
 Done.

Removing the whole test of {{{ if (to < from || from < 0 || to >=
 ItemCount() || ItemCount() == > 0) }}}
sounds a bit dangerous, why not remove just the test for ItemCount() ==
 0 from it?
 I felt it kinda reduntdant..., anyway fix it.


 Also, original branch for someone who lost:
 https://github.com/hyche/haiku/commits/btrfs_review

--
Ticket URL: <https://dev.haiku-os.org/ticket/13612#comment:18>
Haiku <https://dev.haiku-os.org>
The Haiku operating system.

Other related posts: