On 2018-10-13, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > Pardon me, but... huh? The reason for your two calls of dirfd_path_init() is, > > AFAICS, the combination of absolute pathname with both LOOKUP_XDEV and > > LOOKUP_BENEATH at the same time. That combination is treated as if the pathname > > had been relative. Note that LOOKUP_BENEATH alone is ignored for absolute ones > > (and with a good reason - it's a no-op on path_init() level in that case). > > > > What the hell? It complicates your code and doesn't seem to provide any benefits > > whatsoever -- you could bloody well have passed the relative pathname to start with. > > > > IDGI... Without that kludge it becomes simply "do as we currently do for absolute > > pathnames, call dirfd_path_init() for relative ones". And I would argue that > > taking LOOKUP_BENEATH handling out of dirfd_path_init() into path_init() (relative) > > case would be a good idea. > > > > As it is, the logics is very hard to follow. > > ... and it fails on LOOKUP_BENEATH anyway. Egads... So that's for your > LOOKUP_CHROOT ;-/ IMO that's awful, especially with the way you've spread those > LOOKUP_CHROOT cases between these two. Yeah, the ->root setting in dirfd_path_init() is ugly. :/ > Why not simply have O_THISROOT pick root by dirfd and call file_open_root()? Wouldn't this require replicating the dirfd_path_init()-like code inside all of the path_*at() callers which use path_init()? Or is there another common place we could put it? > And if something wants it for stat(), etc. just have them use it combined with > O_PATH and pass the result to ...at()... This works for stat and quite a few other things (which is why I only added openat(2) support for the moment), but I think we'd eventually need something like this for renameat2(2) as well as a few other choice *at(2) syscalls. Though I also think that more AT_EMPTY_PATH support would removed the need for _most_ *at(2) implementations to use this. -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH <https://www.cyphar.com/>
Attachment:
signature.asc
Description: PGP signature
