[systemd-devel] [PATCH v2 1/2] path-util: Fix path_is_mount_point for files

[Lennart Poettering]

On Fri, 29.05.15 17:22, Martin Pitt (martin.pitt at 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