On 6/4/24 09:43, Jemmy wrote:
Hello everyone,
I'm new to Linux kernel development
and excited to make my first contribution.
While working with the copy_tree function,
I noticed some unclear variable names e.g., p, q, r.
I've updated them to be more descriptive,
aiming to make the code easier to understand.
Changes:
p -> o_parent, old parent
q -> n_mnt, new mount
r -> o_mnt, old child
s -> o_child, old child
parent -> n_parent, new parent
Thanks for the opportunity to be part of this community!
BR,
Jemmy
I don't know why I am on the to-list as I haven't worked on this file
before. Anyway, making meaning name is helpful, but I believe a
functional level documentation on top of copy_tree() to explain what
this function tries to accomplish can be helpful too.
Cheers,
Longman
Signed-off-by: Jemmy <jemmywong512@xxxxxxxxx>
---
fs/namespace.c | 51 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 5a51315c6678..b1cf95ddfb87 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1969,7 +1969,7 @@ static bool mnt_ns_loop(struct dentry *dentry)
struct mount *copy_tree(struct mount *mnt, struct dentry *dentry,
int flag)
{
- struct mount *res, *p, *q, *r, *parent;
+ struct mount *res, *o_parent, *o_child, *o_mnt, *n_parent, *n_mnt;
if (!(flag & CL_COPY_UNBINDABLE) && IS_MNT_UNBINDABLE(mnt))
return ERR_PTR(-EINVAL);
@@ -1977,47 +1977,46 @@ struct mount *copy_tree(struct mount *mnt, struct dentry *dentry,
if (!(flag & CL_COPY_MNT_NS_FILE) && is_mnt_ns_file(dentry))
return ERR_PTR(-EINVAL);
- res = q = clone_mnt(mnt, dentry, flag);
- if (IS_ERR(q))
- return q;
+ res = n_mnt = clone_mnt(mnt, dentry, flag);
+ if (IS_ERR(n_mnt))
+ return n_mnt;
- q->mnt_mountpoint = mnt->mnt_mountpoint;
+ n_mnt->mnt_mountpoint = mnt->mnt_mountpoint;
- p = mnt;
- list_for_each_entry(r, &mnt->mnt_mounts, mnt_child) {
- struct mount *s;
- if (!is_subdir(r->mnt_mountpoint, dentry))
+ o_parent = mnt;
+ list_for_each_entry(o_mnt, &mnt->mnt_mounts, mnt_child) {
+ if (!is_subdir(o_mnt->mnt_mountpoint, dentry))
continue;
- for (s = r; s; s = next_mnt(s, r)) {
+ for (o_child = o_mnt; o_child; o_child = next_mnt(o_child, o_mnt)) {
if (!(flag & CL_COPY_UNBINDABLE) &&
- IS_MNT_UNBINDABLE(s)) {
- if (s->mnt.mnt_flags & MNT_LOCKED) {
+ IS_MNT_UNBINDABLE(o_child)) {
+ if (o_child->mnt.mnt_flags & MNT_LOCKED) {
/* Both unbindable and locked. */
- q = ERR_PTR(-EPERM);
+ n_mnt = ERR_PTR(-EPERM);
goto out;
} else {
- s = skip_mnt_tree(s);
+ o_child = skip_mnt_tree(o_child);
continue;
}
}
if (!(flag & CL_COPY_MNT_NS_FILE) &&
- is_mnt_ns_file(s->mnt.mnt_root)) {
- s = skip_mnt_tree(s);
+ is_mnt_ns_file(o_child->mnt.mnt_root)) {
+ o_child = skip_mnt_tree(o_child);
continue;
}
- while (p != s->mnt_parent) {
- p = p->mnt_parent;
- q = q->mnt_parent;
+ while (o_parent != o_child->mnt_parent) {
+ o_parent = o_parent->mnt_parent;
+ n_mnt = n_mnt->mnt_parent;
}
- p = s;
- parent = q;
- q = clone_mnt(p, p->mnt.mnt_root, flag);
- if (IS_ERR(q))
+ o_parent = o_child;
+ n_parent = n_mnt;
+ n_mnt = clone_mnt(o_parent, o_parent->mnt.mnt_root, flag);
+ if (IS_ERR(n_mnt))
goto out;
lock_mount_hash();
- list_add_tail(&q->mnt_list, &res->mnt_list);
- attach_mnt(q, parent, p->mnt_mp, false);
+ list_add_tail(&n_mnt->mnt_list, &res->mnt_list);
+ attach_mnt(n_mnt, n_parent, o_parent->mnt_mp, false);
unlock_mount_hash();
}
}
@@ -2028,7 +2027,7 @@ struct mount *copy_tree(struct mount *mnt, struct dentry *dentry,
umount_tree(res, UMOUNT_SYNC);
unlock_mount_hash();
}
- return q;
+ return n_mnt;
}
/* Caller should check returned pointer for errors */
