Messages in this thread | | | Date | Mon, 28 Oct 2019 02:41:15 +1100 | From | Aleksa Sarai <> | Subject | Re: [PATCH RESEND v14 2/6] namei: LOOKUP_IN_ROOT: chroot-like path resolution |
| |
On 2019-10-27, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Sat, Oct 26, 2019 at 2:58 PM Aleksa Sarai <cyphar@cyphar.com> wrote: > > > > + /* LOOKUP_IN_ROOT treats absolute paths as being relative-to-dirfd. */ > > + if (flags & LOOKUP_IN_ROOT) > > + while (*s == '/') > > + s++; > > + > > /* Figure out the starting path and root (if needed). */ > > if (*s == '/') { > > error = nd_jump_root(nd); > > So I'm still hung up on this. > > I guess I can't help it, but I look at the above, and it makes me go > "whoever wrote those tests wasn't thinking". > > It just annoys me how it tests for '/' completely unnecessarily. > > If LOOKUP_IN_ROOT is true, we know the subsequent test for '/' is not > going to match, because we just removed it. So I look at that code and > go "that code is doing stupid things".
Okay, fair enough.
> That's why I suggested moving the LOOKUP_IN_ROOT check inside the '/' test. > > Alternatively, just make the logic be > > if (flags & LOOKUP_IN_ROOT) { > .. remove '/'s ... > } else if (*s == '/') { > .. handl;e root .. > > and remove the next "else" clause
I've gone with the latter since I think it reads better.
-- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH <https://www.cyphar.com/> [unhandled content-type:application/pgp-signature] | |