On Thu, Aug 28, 2025 at 04:20:56PM -0700, Linus Torvalds wrote:
> (I'll have a separate comment on 61/63)
>
> I certainly agree with the intent of the patch, but that
> can_move_mount_beneath() call line is now rather hard to read. It
> looked simpler before.
>
> Maybe you could just split it into two lines, and write it as
>
> if (beneath) {
> struct mount *new_mnt = real_mount(new_path->mnt);
> err = can_move_mount_beneath(old, new_mnt, mp.mp);
> if (err)
> return err;
> }
>
> which makes slightly less happen in that one line (and it fits in 80
> columns too - not a requirement, but still "good taste")
>
> Long lines are better than randomly splitting lines unreadably into
> multiple lines, but short lines that are logically split are still
> preferred, I would say..
FWIW, if you look at #35, you'll see this:
- err = can_move_mount_beneath(old, real_mount(new_path->mnt), mp.mp);
+ struct mount *over = real_mount(new_path->mnt);
+
+ if (mp.parent != over->mnt_parent)
+ over = mp.parent->overmount;
+ err = can_move_mount_beneath(old, over, mp.mp);
So... might as well introduce the variable in this one.
Then this chunk becomes
@@ -3618,7 +3617,9 @@ static int do_move_mount(struct path *old_path,
}
if (beneath) {
- err = can_move_mount_beneath(old, new_path, mp.mp);
+ struct mount *over = real_mount(new_path->mnt);
+
+ err = can_move_mount_beneath(old, over, mp.mp);
if (err)
return err;
}
and the corresponding one in #35
@@ -3618,6 +3624,8 @@ static int do_move_mount(struct path *old_path,
if (beneath) {
struct mount *over = real_mount(new_path->mnt);
+ if (mp.parent != over->mnt_parent)
+ over = mp.parent->overmount;
err = can_move_mount_beneath(old, over, mp.mp);
if (err)
return err;
OK, done - both certainly look better that way.
