ping. On Sun, 13 Sep 2015 14:50:19 +0100 bytevolc...@safe-mail.net wrote:
> Hello, > > Although a patch like this was submitted before, this one is more > "focused". It may be useful for those moving from MFS who use this > option to pre-populate an MFS mount with a collection of files. This > will refuse to work if the path specified for -P is not a directory. > If the copy is unsuccessful, unmount and inform the user. > > A few questions on this though: > > 1. Is there a better way than executing pax, to copy all the files in > a directory? > > 2. The do_exec() function was sort of copied from sbin/newfs/newfs.c. > It doesn't feel like the best way of handling this, but some research > suggests it is better than calling system() due to security issues, > one that springs to mind is a malformed input directory to the -P > option. Is there a simple way to sanitise the input for use with > system()? > > 3. I noticed a pattern of functions that have about 2-3 lines of code > in them. Is it best to keep things that way, or is it best to merge > them into a single function (eg. copy_dir() merged into the > mount_tmpfs() function)? > > 4. Are variable declarations to be at the beginning of a function > block, or the beginning of the immediate block? I feel limiting scope > is a good idea, but would like some feedback on this as I cannot see > anything in style(9). > > 5. Is mount_tmpfs.h really necessary, given nothing in the tree seems > to use anything related to the mount_tmpfs() function which is > internal to the mount_tmpfs binary? > > > --------- > > Index: sbin/mount_tmpfs/mount_tmpfs.8 > =================================================================== > RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v > retrieving revision 1.4 > diff -u -p -r1.4 mount_tmpfs.8 > --- sbin/mount_tmpfs/mount_tmpfs.8 16 Nov 2014 02:22:10 > -0000 1.4 +++ sbin/mount_tmpfs/mount_tmpfs.8 13 Sep > 2015 13:03:34 -0000 @@ -41,6 +41,7 @@ > .Op Fl m Ar mode > .Op Fl n Ar nodes > .Op Fl o Ar options > +.Op Fl P Ar directory > .Op Fl s Ar size > .Op Fl u Ar user > .Ar tmpfs > @@ -80,6 +81,8 @@ flag followed by a comma-separated strin > See the > .Xr mount 8 > man page for possible options and their meanings. > +.It Fl P Ar directory > +Populate the created tmpfs file system with the contents of the > directory. .It Fl s Ar size > Specifies the total file system size in bytes. > If zero is given (the default), the available amount of memory > (including @@ -136,6 +139,10 @@ and > .Ox 5.5 . > .Sh CAVEATS > The update of mount options (through mount -u) is currently not > supported. +The > +.Fl P > +option will produce an error if the mount is read-only, or if the > files +cannot be copied from the specified template directory. > .Sh BUGS > File system meta-data is not pageable. > If there is not enough main memory to hold this information, the > system may Index: sbin/mount_tmpfs/mount_tmpfs.c > =================================================================== > RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v > retrieving revision 1.5 > diff -u -p -r1.5 mount_tmpfs.c > --- sbin/mount_tmpfs/mount_tmpfs.c 16 Jan 2015 06:39:59 > -0000 1.5 +++ sbin/mount_tmpfs/mount_tmpfs.c 13 Sep > 2015 13:03:34 -0000 @@ -34,6 +34,7 @@ > #include <sys/types.h> > #include <sys/mount.h> > #include <sys/stat.h> > +#include <sys/wait.h> > > #include <ctype.h> > #include <err.h> > @@ -41,6 +42,7 @@ > #include <grp.h> > #include <mntopts.h> > #include <pwd.h> > +#include <signal.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > @@ -68,12 +70,15 @@ static int a_num(const char *, const cha > static mode_t a_mask(const char *); > static void pathadj(const char *, char *); > > +static int do_exec(const char *, const char *, char *const[]); > +static int copy_dir(char *, char *); > + > /* > --------------------------------------------------------------------- > */ void > mount_tmpfs_parseargs(int argc, char *argv[], > struct tmpfs_args *args, int *mntflags, > - char *canon_dev, char *canon_dir) > + char *canon_dev, char *canon_dir, char *pop_dir) > { > int gidset, modeset, uidset; /* Ought to be 'bool'. */ > int ch; > @@ -95,7 +100,7 @@ mount_tmpfs_parseargs(int argc, char *ar > modeset = 0; mode = 0; > > optind = optreset = 1; > - while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) { > + while ((ch = getopt(argc, argv, "P:g:m:n:o:s:u:")) != -1 ) { > switch (ch) { > case 'g': > gid = a_gid(optarg); > @@ -131,6 +136,10 @@ mount_tmpfs_parseargs(int argc, char *ar > uidset = 1; > break; > > + case 'P': > + strlcpy(pop_dir, optarg, PATH_MAX); > + break; > + > case '?': > default: > usage(); > @@ -161,7 +170,8 @@ usage(void) > extern char *__progname; > (void)fprintf(stderr, > "usage: %s [-g group] [-m mode] [-n nodes] [-o options] > [-s size]\n" > - " [-u user] tmpfs mount_point\n", __progname); > + " [-P directory] [-u user] tmpfs > mount_point\n", > + __progname); > exit(1); > } > > @@ -172,14 +182,33 @@ mount_tmpfs(int argc, char *argv[]) > { > struct tmpfs_args args; > char canon_dev[PATH_MAX], canon_dir[PATH_MAX]; > + char pop_dir[PATH_MAX] = {0}; > int mntflags; > + int ret; > + struct stat st; > > mount_tmpfs_parseargs(argc, argv, &args, &mntflags, > - canon_dev, canon_dir); > + canon_dev, canon_dir, pop_dir); > + > + if(pop_dir[0] != '\0') { > + if (stat(pop_dir, &st) != 0) > + err(1, "cannot stat %s", pop_dir); > + if (!S_ISDIR(st.st_mode)) > + errx(1, "%s: not a directory", pop_dir); > + } > > if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) == -1) > err(EXIT_FAILURE, "tmpfs on %s", canon_dir); > > + if (pop_dir[0] != '\0') { > + ret = copy_dir(pop_dir, canon_dir); > + if (ret != 0) { > + if (unmount(canon_dir, 0) != 0) > + warn("unmount %s", canon_dir); > + errx(1, "copy %s to %s failed", pop_dir, > canon_dir); > + } > + } > + > return EXIT_SUCCESS; > } > > @@ -245,4 +274,55 @@ pathadj(const char *input, char *adjuste > warnx("\"%s\" is a non-resolved or relative path.", > input); warnx("using \"%s\" instead.", adjusted); > } > +} > + > +static int > +do_exec(const char *dir, const char *cmd, char *const argv[]) > +{ > + pid_t pid; > + int ret, status; > + sig_t intsave, quitsave; > + > + switch (pid = fork()) { > + case -1: > + err(1, "fork"); > + case 0: > + if (dir != NULL && chdir(dir) != 0) > + err(1, "chdir"); > + if (execv(cmd, argv) != 0) > + err(1, "%s", cmd); > + break; > + default: > + intsave = signal(SIGINT, SIG_IGN); > + quitsave = signal(SIGQUIT, SIG_IGN); > + for (;;) { > + ret = waitpid(pid, &status, 0); > + if (ret == -1) > + err(11, "waitpid"); > + if (WIFEXITED(status)) { > + status = WEXITSTATUS(status); > + if (status != 0) > + warnx("%s: exited", cmd); > + break; > + } else if (WIFSIGNALED(status)) { > + warnx("%s: %s", cmd, > + strsignal(WTERMSIG(status))); > + status = 1; > + break; > + } > + } > + signal(SIGINT, intsave); > + signal(SIGQUIT, quitsave); > + return (status); > + } > + /* NOTREACHED */ > + return (-1); > +} > + > +static int > +copy_dir(char *src, char *dst) > +{ > + char *const argv[] = { "pax", "-rw", "-pe", ".", dst, > NULL } ; + > + return do_exec(src, "/bin/pax", argv); > } > Index: sbin/mount_tmpfs/mount_tmpfs.h > =================================================================== > RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.h,v > retrieving revision 1.2 > diff -u -p -r1.2 mount_tmpfs.h > --- sbin/mount_tmpfs/mount_tmpfs.h 3 Jun 2013 10:37:02 > -0000 1.2 +++ sbin/mount_tmpfs/mount_tmpfs.h 13 Sep > 2015 13:03:34 -0000 @@ -31,6 +31,6 @@ > > int mount_tmpfs(int, char **); > void mount_tmpfs_parseargs(int, char **, struct tmpfs_args *, > int *, > - char *, char *); > + char *, char *, char *); > > #endif /* _SBIN_MOUNT_TMPFS_MOUNT_TMPFS_H_ */