On Fri, 29.05.15 17:22, Martin Pitt (martin.p...@ubuntu.com) wrote:
> Hello,
>
> Lennart Poettering [2015-05-28 19:44 +0200]:
> > I really think this should work as close as the usual *at() calls
> > work. i.e. take a dir fd as first argument, and a filename
> > *within*that*directory* to check. Maybe even give it the _at() suffix:
> >
> > int fd_is_mount_point_at(int fd, const char *filename, int flags);
> > int path_is_mount_point(const char *path, int flags);
> >
> > path_is_mount_point() simply seperates the last part of the path,
> > opens its parent directory, and then invokes fd_is_mount_point_at()
> > with the parent dir and the last component...
>
> Done now, this indeed looks much better now, avoids the "have parent
> or not" cases.
>
> This patch keeps the signature of path_is_mount_point(), as that's
> being used in a lot of places. For simpler revivewing I'll send that
> as a second patch. This updates fd_is_mount_point_at() to behave like
> openat() and work for files again, fixing the regression.
One minor nitpick:
>
> fallback_fstat:
> - if (fstatat(fd, "", &a, AT_EMPTY_PATH) < 0)
> + /* yay for fstatat() taking a different set of flags than the other
> + * _at() above */
> + if (flags & AT_SYMLINK_FOLLOW)
> + flags = flags & ~AT_SYMLINK_FOLLOW;
> + else
> + flags |= AT_SYMLINK_NOFOLLOW;
> + if (fstatat(fd, filename, &a, flags) < 0)
> return -errno;
I think this:
flags = flags & ~AT_SYMLINK_FOLLOW;
should better be written like this:
flags &= ~AT_SYMLINK_FOLLOW;
this matches the other if branch better then...
Looks good otherwise, please push.
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
