Message ID | 5ecbb3082f16956d049cfa98662f8e3384a6aea2.1665326258.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fsmonitor: Implement fsmonitor for Linux | expand |
On Sun, Oct 09 2022, Eric DeCosta via GitGitGadget wrote: > From: Eric DeCosta <edecosta@mathworks.com> > > Compare the given path to the mounted filesystems. Find the mount that is > the longest prefix of the path (if any) and determine if that mount is on a > local or remote filesystem. > > Signed-off-by: Eric DeCosta <edecosta@mathworks.com> > --- > compat/fsmonitor/fsm-path-utils-linux.c | 163 ++++++++++++++++++++++++ > 1 file changed, 163 insertions(+) > create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c > > diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c > new file mode 100644 > index 00000000000..369692a788f > --- /dev/null > +++ b/compat/fsmonitor/fsm-path-utils-linux.c > @@ -0,0 +1,163 @@ > +#include "fsmonitor.h" > +#include "fsmonitor-path-utils.h" > +#include <errno.h> > +#include <mntent.h> > +#include <sys/mount.h> > +#include <sys/statvfs.h> > + > +/* > + * https://github.com/coreutils/gnulib/blob/master/lib/mountlist.c > + */ > +#ifndef ME_REMOTE > +/* A file system is "remote" if its Fs_name contains a ':' > + or if (it is of type (smbfs or cifs) and its Fs_name starts with '//') > + or if it is of any other of the listed types > + or Fs_name is equal to "-hosts" (used by autofs to mount remote fs). > + "VM" file systems like prl_fs or vboxsf are not considered remote here. */ > +# define ME_REMOTE(Fs_name, Fs_type) \ > + (strchr (Fs_name, ':') != NULL \ > + || ((Fs_name)[0] == '/' \ > + && (Fs_name)[1] == '/' \ > + && (strcmp (Fs_type, "smbfs") == 0 \ > + || strcmp (Fs_type, "smb3") == 0 \ > + || strcmp (Fs_type, "cifs") == 0)) \ > + || strcmp (Fs_type, "acfs") == 0 \ > + || strcmp (Fs_type, "afs") == 0 \ > + || strcmp (Fs_type, "coda") == 0 \ > + || strcmp (Fs_type, "auristorfs") == 0 \ > + || strcmp (Fs_type, "fhgfs") == 0 \ > + || strcmp (Fs_type, "gpfs") == 0 \ > + || strcmp (Fs_type, "ibrix") == 0 \ > + || strcmp (Fs_type, "ocfs2") == 0 \ > + || strcmp (Fs_type, "vxfs") == 0 \ > + || strcmp ("-hosts", Fs_name) == 0) > +#endif So, this is just copy/pasted GPLv3 code into our GPLv2-only codebase?: https://github.com/coreutils/gnulib/blob/cd1fdabe8b66c102124b6a5b0c97dded20246b46/lib/mountlist.c#L230-L247 > +static struct mntent *find_mount(const char *path, const struct statvfs *fs) > +{ > + const char *const mounts = "/proc/mounts"; > + const char *rp = real_pathdup(path, 1); > + struct mntent *ment = NULL; > + struct mntent *found = NULL; > + struct statvfs mntfs; > + FILE *fp; > + int dlen, plen, flen = 0; > + > + fp = setmntent(mounts, "r"); > + if (!fp) { > + error_errno(_("setmntent('%s') failed"), mounts); > + return NULL; This code would probably be nicer if you returned int, and passed a pointer to a struct to populate as a paremeter. Then you could just "return error..." for this and the below cases. > + } > + > + plen = strlen(rp); > + > + /* read all the mount information and compare to path */ > + while ((ment = getmntent(fp)) != NULL) { > + if (statvfs(ment->mnt_dir, &mntfs)) { > + switch (errno) { > + case EPERM: > + case ESRCH: > + case EACCES: > + continue; > + default: > + error_errno(_("statvfs('%s') failed"), ment->mnt_dir); > + endmntent(fp);is > + return NULL; > + } > + } > + > + /* is mount on the same filesystem and is a prefix of the path */ > + if ((fs->f_fsid == mntfs.f_fsid) && > + !strncmp(ment->mnt_dir, rp, strlen(ment->mnt_dir))) { > + dlen = strlen(ment->mnt_dir); > + if (dlen > plen) > + continue; > + /* > + * root is always a potential match; otherwise look for > + * directory prefix > + */ > + if ((dlen == 1 && ment->mnt_dir[0] == '/') || > + (dlen > flen && (!rp[dlen] || rp[dlen] == '/'))) { > + flen = dlen; > + /* > + * https://man7.org/linux/man-pages/man3/getmntent.3.html > + * > + * The pointer points to a static area of memory which is > + * overwritten by subsequent calls to getmntent(). > + */ > + if (!found) > + CALLOC_ARRAY(found, 1); It seems we never populate >1 of these, so don't you just want xmalloc(). Or actually... > + free(found->mnt_dir); > + free(found->mnt_type); > + free(found->mnt_fsname); > + found->mnt_dir = xmemdupz(ment->mnt_dir, strlen(ment->mnt_dir)); > + found->mnt_type = xmemdupz(ment->mnt_type, strlen(ment->mnt_type)); > + found->mnt_fsname = xmemdupz(ment->mnt_fsname, strlen(ment->mnt_fsname)); Don't mix mem*() and str*(). In this case we need the string to be '\0' delimited, so use xstrndup(). And once we do that, we might wonder why we're explicitly finding the '\0', just to pass it to the xstrn*() function, when we can just do: found->mnt_dir = xstrdup(ment->mnt_dir); ... Which would AFAICT be equivalent to what you're doing here. > + } > + } > + } > + endmntent(fp); > + > + return found; > +} > + > +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info) > +{ > + struct mntent *ment; ...make this (or above) a "struct mntent ment", then pass &ment down, so we can fill that struct? Dunno... > + struct statvfs fs; > + > + if (statvfs(path, &fs)) > + return error_errno(_("statvfs('%s') failed"), path); > + > + ment = find_mount(path, &fs); > + if (!ment) > + return -1; > + > + trace_printf_key(&trace_fsmonitor, > + "statvfs('%s') [flags 0x%08lx] '%s' '%s'", > + path, fs.f_flag, ment->mnt_type, ment->mnt_fsname); > + > + if (ME_REMOTE(ment->mnt_fsname, ment->mnt_type)) > + fs_info->is_remote = 1; > + else > + fs_info->is_remote = 0; Shorter: fs_info->is_remote = ME_REMOTE(ment->mnt_fsname, ment->mnt_type);
> -----Original Message----- > From: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > Sent: Monday, October 10, 2022 6:04 AM > To: Eric DeCosta via GitGitGadget <gitgitgadget@gmail.com> > Cc: git@vger.kernel.org; Eric DeCosta <edecosta@mathworks.com> > Subject: Re: [PATCH 08/12] fsmonitor: determine if filesystem is local or > remote > > > On Sun, Oct 09 2022, Eric DeCosta via GitGitGadget wrote: > > > From: Eric DeCosta <edecosta@mathworks.com> > > > > Compare the given path to the mounted filesystems. Find the mount that > > is the longest prefix of the path (if any) and determine if that mount > > is on a local or remote filesystem. > > > > Signed-off-by: Eric DeCosta <edecosta@mathworks.com> > > --- > > compat/fsmonitor/fsm-path-utils-linux.c | 163 > ++++++++++++++++++++++++ > > 1 file changed, 163 insertions(+) > > create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c > > > > diff --git a/compat/fsmonitor/fsm-path-utils-linux.c > > b/compat/fsmonitor/fsm-path-utils-linux.c > > new file mode 100644 > > index 00000000000..369692a788f > > --- /dev/null > > +++ b/compat/fsmonitor/fsm-path-utils-linux.c > > @@ -0,0 +1,163 @@ > > +#include "fsmonitor.h" > > +#include "fsmonitor-path-utils.h" > > +#include <errno.h> > > +#include <mntent.h> > > +#include <sys/mount.h> > > +#include <sys/statvfs.h> > > + > > +/* > > + * https://github.com/coreutils/gnulib/blob/master/lib/mountlist.c > > +<https://protect- > us.mimecast.com/s/wKJsC9rj7lc3WKmYUOq9E9?domain=gith > > +ub.com> > > + */ > > +#ifndef ME_REMOTE > > +/* A file system is "remote" if its Fs_name contains a ':' > > + or if (it is of type (smbfs or cifs) and its Fs_name starts with > > +'//') or if it is of any other of the listed types or Fs_name is > > +equal to "-hosts" (used by autofs to mount remote fs). > > + "VM" file systems like prl_fs or vboxsf are not considered remote > > +here. */ # define ME_REMOTE(Fs_name, Fs_type) \ (strchr (Fs_name, > > +':') != NULL \ > > + || ((Fs_name)[0] == '/' \ > > + && (Fs_name)[1] == '/' \ > > + && (strcmp (Fs_type, "smbfs") == 0 \ > > + || strcmp (Fs_type, "smb3") == 0 \ > > + || strcmp (Fs_type, "cifs") == 0)) \ strcmp (Fs_type, "acfs") == 0 \ > > + || strcmp (Fs_type, "afs") == 0 \ strcmp (Fs_type, "coda") == 0 \ > > + || strcmp (Fs_type, "auristorfs") == 0 \ strcmp (Fs_type, "fhgfs") > > + || == 0 \ strcmp (Fs_type, "gpfs") == 0 \ strcmp (Fs_type, "ibrix") > > + || == 0 \ strcmp (Fs_type, "ocfs2") == 0 \ strcmp (Fs_type, "vxfs") > > + || == 0 \ strcmp ("-hosts", Fs_name) == 0) > > +#endif > > So, this is just copy/pasted GPLv3 code into our GPLv2-only codebase?: > https://github.com/coreutils/gnulib/blob/cd1fdabe8b66c102124b6a5b0c97d > ded20246b46/lib/mountlist.c#L230-L247 <https://protect- > us.mimecast.com/s/zxPbC0RMy1hoXW2rCOSXJD?domain=github.com> > Yes. I was hoping for some guidance as to what to do with ME_REMOTE. I also found it, verbatim, here in midnight commander: https://github.com/MidnightCommander/mc/blob/e48cd98ac13a9b4366bd88287f632413766b967f/src/filemanager/mountlist.c#L258-L281 But that's just another GPLv3 code base. > > +static struct mntent *find_mount(const char *path, const struct > > +statvfs *fs) { const char *const mounts = "/proc/mounts"; const > > +char *rp = real_pathdup(path, 1); struct mntent *ment = NULL; > > +struct mntent *found = NULL; struct statvfs mntfs; FILE *fp; int > > +dlen, plen, flen = 0; > > + > > + fp = setmntent(mounts, "r"); > > + if (!fp) { > > + error_errno(_("setmntent('%s') failed"), mounts); return NULL; > > This code would probably be nicer if you returned int, and passed a pointer > to a struct to populate as a paremeter. Then you could just "return error..." > for this and the below cases. > > > + } > > + > > + plen = strlen(rp); > > + > > + /* read all the mount information and compare to path */ while > > + ((ment = getmntent(fp)) != NULL) { if (statvfs(ment->mnt_dir, > > + &mntfs)) { switch (errno) { case EPERM: > > + case ESRCH: > > + case EACCES: > > + continue; > > + default: > > + error_errno(_("statvfs('%s') failed"), ment->mnt_dir); > > + endmntent(fp);is return NULL; } } > > + > > + /* is mount on the same filesystem and is a prefix of the path */ if > > + ((fs->f_fsid == mntfs.f_fsid) && !strncmp(ment->mnt_dir, rp, > > + strlen(ment->mnt_dir))) { dlen = strlen(ment->mnt_dir); if (dlen > > > + plen) continue; > > + /* > > + * root is always a potential match; otherwise look for > > + * directory prefix > > + */ > > + if ((dlen == 1 && ment->mnt_dir[0] == '/') || (dlen > flen && > > + (!rp[dlen] || rp[dlen] == '/'))) { flen = dlen; > > + /* > > + * https://man7.org/linux/man-pages/man3/getmntent.3.html > > + <https://protect- > us.mimecast.com/s/aOmSCgJyVrT01WlYc76tSR?domain=man > > + 7.org> > > + * > > + * The pointer points to a static area of memory which is > > + * overwritten by subsequent calls to getmntent(). > > + */ > > + if (!found) > > + CALLOC_ARRAY(found, 1); > > It seems we never populate >1 of these, so don't you just want xmalloc(). Or > actually... > > > + free(found->mnt_dir); > > + free(found->mnt_type); > > + free(found->mnt_fsname); > > + found->mnt_dir = xmemdupz(ment->mnt_dir, strlen(ment->mnt_dir)); > > + found->mnt_type = xmemdupz(ment->mnt_type, strlen(ment- > >mnt_type)); > > + found->mnt_fsname = xmemdupz(ment->mnt_fsname, > > + found->strlen(ment->mnt_fsname)); > > Don't mix mem*() and str*(). In this case we need the string to be '\0' > delimited, so use xstrndup(). > > And once we do that, we might wonder why we're explicitly finding the '\0', > just to pass it to the xstrn*() function, when we can just do: > > found->mnt_dir = xstrdup(ment->mnt_dir); > ... > > Which would AFAICT be equivalent to what you're doing here. > > > + } > > + } > > + } > > + endmntent(fp); > > + > > + return found; > > +} > > + > > +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info) > > +{ struct mntent *ment; > > ...make this (or above) a "struct mntent ment", then pass &ment down, so > we can fill that struct? Dunno... > > > + struct statvfs fs; > > + > > + if (statvfs(path, &fs)) > > + return error_errno(_("statvfs('%s') failed"), path); > > + > > + ment = find_mount(path, &fs); > > + if (!ment) > > + return -1; > > + > > + trace_printf_key(&trace_fsmonitor, > > + "statvfs('%s') [flags 0x%08lx] '%s' '%s'", path, fs.f_flag, > > + ment->mnt_type, ment->mnt_fsname); > > + > > + if (ME_REMOTE(ment->mnt_fsname, ment->mnt_type)) fs_info- > >is_remote > > + = 1; else fs_info->is_remote = 0; > > Shorter: > > fs_info->is_remote = ME_REMOTE(ment->mnt_fsname, ment->mnt_type);
diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c new file mode 100644 index 00000000000..369692a788f --- /dev/null +++ b/compat/fsmonitor/fsm-path-utils-linux.c @@ -0,0 +1,163 @@ +#include "fsmonitor.h" +#include "fsmonitor-path-utils.h" +#include <errno.h> +#include <mntent.h> +#include <sys/mount.h> +#include <sys/statvfs.h> + +/* + * https://github.com/coreutils/gnulib/blob/master/lib/mountlist.c + */ +#ifndef ME_REMOTE +/* A file system is "remote" if its Fs_name contains a ':' + or if (it is of type (smbfs or cifs) and its Fs_name starts with '//') + or if it is of any other of the listed types + or Fs_name is equal to "-hosts" (used by autofs to mount remote fs). + "VM" file systems like prl_fs or vboxsf are not considered remote here. */ +# define ME_REMOTE(Fs_name, Fs_type) \ + (strchr (Fs_name, ':') != NULL \ + || ((Fs_name)[0] == '/' \ + && (Fs_name)[1] == '/' \ + && (strcmp (Fs_type, "smbfs") == 0 \ + || strcmp (Fs_type, "smb3") == 0 \ + || strcmp (Fs_type, "cifs") == 0)) \ + || strcmp (Fs_type, "acfs") == 0 \ + || strcmp (Fs_type, "afs") == 0 \ + || strcmp (Fs_type, "coda") == 0 \ + || strcmp (Fs_type, "auristorfs") == 0 \ + || strcmp (Fs_type, "fhgfs") == 0 \ + || strcmp (Fs_type, "gpfs") == 0 \ + || strcmp (Fs_type, "ibrix") == 0 \ + || strcmp (Fs_type, "ocfs2") == 0 \ + || strcmp (Fs_type, "vxfs") == 0 \ + || strcmp ("-hosts", Fs_name) == 0) +#endif + +static struct mntent *find_mount(const char *path, const struct statvfs *fs) +{ + const char *const mounts = "/proc/mounts"; + const char *rp = real_pathdup(path, 1); + struct mntent *ment = NULL; + struct mntent *found = NULL; + struct statvfs mntfs; + FILE *fp; + int dlen, plen, flen = 0; + + fp = setmntent(mounts, "r"); + if (!fp) { + error_errno(_("setmntent('%s') failed"), mounts); + return NULL; + } + + plen = strlen(rp); + + /* read all the mount information and compare to path */ + while ((ment = getmntent(fp)) != NULL) { + if (statvfs(ment->mnt_dir, &mntfs)) { + switch (errno) { + case EPERM: + case ESRCH: + case EACCES: + continue; + default: + error_errno(_("statvfs('%s') failed"), ment->mnt_dir); + endmntent(fp); + return NULL; + } + } + + /* is mount on the same filesystem and is a prefix of the path */ + if ((fs->f_fsid == mntfs.f_fsid) && + !strncmp(ment->mnt_dir, rp, strlen(ment->mnt_dir))) { + dlen = strlen(ment->mnt_dir); + if (dlen > plen) + continue; + /* + * root is always a potential match; otherwise look for + * directory prefix + */ + if ((dlen == 1 && ment->mnt_dir[0] == '/') || + (dlen > flen && (!rp[dlen] || rp[dlen] == '/'))) { + flen = dlen; + /* + * https://man7.org/linux/man-pages/man3/getmntent.3.html + * + * The pointer points to a static area of memory which is + * overwritten by subsequent calls to getmntent(). + */ + if (!found) + CALLOC_ARRAY(found, 1); + free(found->mnt_dir); + free(found->mnt_type); + free(found->mnt_fsname); + found->mnt_dir = xmemdupz(ment->mnt_dir, strlen(ment->mnt_dir)); + found->mnt_type = xmemdupz(ment->mnt_type, strlen(ment->mnt_type)); + found->mnt_fsname = xmemdupz(ment->mnt_fsname, strlen(ment->mnt_fsname)); + } + } + } + endmntent(fp); + + return found; +} + +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info) +{ + struct mntent *ment; + struct statvfs fs; + + if (statvfs(path, &fs)) + return error_errno(_("statvfs('%s') failed"), path); + + ment = find_mount(path, &fs); + if (!ment) + return -1; + + trace_printf_key(&trace_fsmonitor, + "statvfs('%s') [flags 0x%08lx] '%s' '%s'", + path, fs.f_flag, ment->mnt_type, ment->mnt_fsname); + + if (ME_REMOTE(ment->mnt_fsname, ment->mnt_type)) + fs_info->is_remote = 1; + else + fs_info->is_remote = 0; + + fs_info->typename = ment->mnt_fsname; + free(ment->mnt_dir); + free(ment->mnt_type); + free(ment); + + trace_printf_key(&trace_fsmonitor, + "'%s' is_remote: %d", + path, fs_info->is_remote); + return 0; +} + +int fsmonitor__is_fs_remote(const char *path) +{ + struct fs_info fs; + + if (fsmonitor__get_fs_info(path, &fs)) + return -1; + + free(fs.typename); + + return fs.is_remote; +} + +/* + * No-op for now. + */ +int fsmonitor__get_alias(const char *path, struct alias_info *info) +{ + return 0; +} + +/* + * No-op for now. + */ +char *fsmonitor__resolve_alias(const char *path, + const struct alias_info *info) +{ + return NULL; +}