On Wed, Dec 28, 2016 at 11:51 PM, Adrien Destugues
<pulkomandy@xxxxxxxxxxxxx> wrote:
Orthograph: increment/decrement, not incriment/decriment (several places
in comments too)
diff --git a/src/tools/fs_shell/vfs.cpp b/src/tools/fs_shell/vfs.cpp
index 5137ddd..cb7e654 100644
--- a/src/tools/fs_shell/vfs.cpp
+++ b/src/tools/fs_shell/vfs.cpp
@@ -1054,7 +1054,14 @@ entry_ref_to_vnode(fssh_mount_id mountID,
fssh_vnode_id directoryID, const char
if (status < 0)
return status;
- return vnode_path_to_vnode(directory, clonedName, false, 0, _vnode, NULL);
+ inc_vnode_ref_count(directory);
+ // incriment directory ref_count before calling vnode_path_to_vnode()
+ status = vnode_path_to_vnode(directory, clonedName, false, 0, _vnode,
NULL);
+ // vnode_path_to_vnode() decriments directory ref_count
+ put_vnode(directory);
+ // decriment directory ref_count again, freeing it but not _vnode
This change looks useless, the vnode is freed immediately anyway.
I wonder if it is possible to make vnode_path_to_vnode "balanced" (not
changing the ref_count inside it). I guess it was meant to be used as a
way to "convert directory to one of its leaves", which can be useful
when browsing the directory hierarchy, but it is now used in cases where
the directory is still used after calling it, so that would need to be
redesigned.
It would make the API behave more in the expected way, and would avoid
that the error happens again in the future.
I would say vnode can't be null at these points. It isn't checked
directly, but there is a check for "status" before which make sure the
functions initializing it ran properly. If you want to check anyway, an
assert() would be a better choice here, I think.
