Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-21 Thread Adam
I apologise to all (especially Theo) for my behavior and remarks here,
and I apologise if I came across as being arrogant and unwilling to
cooperate.
I am going to leave for at least 4 weeks to cool off before I post
again.

Once again, I am sorry.

 - Adam.



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation. Mk II.

2014-09-20 Thread Adam
I got of to a bit of a bad start with this patch, and things were
allover the place. I knew it was bad when Theo gave me a bit of a boot.

And so now I think it is time for Mk II.
Hopefully I don't make an ass out of myself this time.

I eliminated some unused definitions that were added in Rev 5.


Latest revision of the patch below:



Index: sys/tmpfs/tmpfs_vfsops.c
===
RCS file: /cvs/src/sys/tmpfs/tmpfs_vfsops.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 tmpfs_vfsops.c
--- sys/tmpfs/tmpfs_vfsops.c12 Jul 2014 18:50:25 -  1.4
+++ sys/tmpfs/tmpfs_vfsops.c20 Sep 2014 08:48:40 -
@@ -118,7 +118,21 @@ tmpfs_mount(struct mount *mp, const char
}
 #endif
 
+   /*
+* If updating, check whether changing from read-only to
+* read/write; if there is no device name, that's all we do.
+*/
if (mp->mnt_flag & MNT_UPDATE) {
+
+   /* Update if changing the read-only flag. */
+   if(mp->mnt_flag & MNT_RDONLY)
+   return 0;
+
+   if(mp->mnt_flag & MNT_WANTRDWR) {
+   mp->mnt_flag &= ~(MNT_WANTRDWR | MNT_RDONLY);
+   return 0;
+   }
+
/* TODO */
return EOPNOTSUPP;
}
Index: sbin/mount_tmpfs/mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -u -r1.3 mount_tmpfs.8
--- sbin/mount_tmpfs/mount_tmpfs.8  5 Feb 2014 15:32:26 -   1.3
+++ sbin/mount_tmpfs/mount_tmpfs.8  20 Sep 2014 08:48:41 -
@@ -43,6 +43,7 @@
 .Op Fl o Ar options
 .Op Fl s Ar size
 .Op Fl u Ar user
+.Op Fl P Ar template
 .Ar tmpfs
 .Ar mount_point
 .Sh DESCRIPTION
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl P Ar template
+If
+.Ar template
+is a directory, populate the created tmpfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created tmpfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl P Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing
Index: sbin/mount_tmpfs/mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mount_tmpfs.c
--- sbin/mount_tmpfs/mount_tmpfs.c  21 Jan 2014 21:58:27 -  1.4
+++ sbin/mount_tmpfs/mount_tmpfs.c  20 Sep 2014 08:48:41 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,6 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
+
+#include "pathnames.h"
 
 #include "mount_tmpfs.h"
 
@@ -74,10 +78,17 @@ static void pathadj(const char *, char *
 
 /* - */
 
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+   char *template, char *canon_dir)
 {
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
@@ -99,7 +110,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, "g:m:n:o:s:u:P:")) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -134,6 +145,12 @@ mount_tmpfs_parseargs(int argc, char *ar
uid = a_uid(optarg);
uidset = 1;
break;
+   
+   case 'P':
+   if(strlcpy(template, optarg, MAXPATHLEN)
+   >= MAXPATHLEN)
+   errx(1, "template path %s too long", optarg);
+   break;
 
case '?':
default:
@@ -146,7 +163,6 @@ mount_tmpfs_parseargs(int argc, char *ar
if (argc != 2)

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
On Fri, 19 Sep 2014 16:08:33 +0200
Otto Moerbeek  wrote:

> On Fri, Sep 19, 2014 at 10:21:13PM +1000, Adam wrote:
> 
> > Patch for argument parsing below.
> > 
> > Diff taken in relation to Revision 3 of the aforementioned patch.
> > 
> > I do not understand why this should be a separate diff for this, as
> > it is essentially part of the functionality itself.
> > 
> > Let me know if you want it all together.
> 
> Well, this is not what I meant.
> 
> I meant: if you want to restructure, do not mix that with the
> additiont of functionality. 
> 
>   -Otto

In this case, Revision 4 (the whole lot together) below:




Index: sys/tmpfs/tmpfs_vfsops.c
===
RCS file: /cvs/src/sys/tmpfs/tmpfs_vfsops.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 tmpfs_vfsops.c
--- sys/tmpfs/tmpfs_vfsops.c12 Jul 2014 18:50:25 -  1.4
+++ sys/tmpfs/tmpfs_vfsops.c19 Sep 2014 21:35:56 -
@@ -118,7 +118,21 @@ tmpfs_mount(struct mount *mp, const char
}
 #endif
 
+   /*
+* If updating, check whether changing from read-only to
+* read/write; if there is no device name, that's all we do.
+*/
if (mp->mnt_flag & MNT_UPDATE) {
+
+   /* Update if changing the read-only flag. */
+   if(mp->mnt_flag & MNT_RDONLY)
+   return 0;
+
+   if(mp->mnt_flag & MNT_WANTRDWR) {
+   mp->mnt_flag &= ~(MNT_WANTRDWR | MNT_RDONLY);
+   return 0;
+   }
+
/* TODO */
return EOPNOTSUPP;
}
Index: sbin/mount_tmpfs/mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -u -r1.3 mount_tmpfs.8
--- sbin/mount_tmpfs/mount_tmpfs.8  5 Feb 2014 15:32:26 -   1.3
+++ sbin/mount_tmpfs/mount_tmpfs.8  19 Sep 2014 21:35:56 -
@@ -43,6 +43,7 @@
 .Op Fl o Ar options
 .Op Fl s Ar size
 .Op Fl u Ar user
+.Op Fl P Ar template
 .Ar tmpfs
 .Ar mount_point
 .Sh DESCRIPTION
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl P Ar template
+If
+.Ar template
+is a directory, populate the created tmpfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created tmpfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl P Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing
Index: sbin/mount_tmpfs/mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mount_tmpfs.c
--- sbin/mount_tmpfs/mount_tmpfs.c  21 Jan 2014 21:58:27 -  1.4
+++ sbin/mount_tmpfs/mount_tmpfs.c  19 Sep 2014 21:35:56 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,6 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
+
+#include "pathnames.h"
 
 #include "mount_tmpfs.h"
 
@@ -74,10 +78,17 @@ static void pathadj(const char *, char *
 
 /* - */
 
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+   char *template, char *canon_dir)
 {
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
@@ -99,7 +110,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, "g:m:n:o:s:u:P:")) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -134,6 +145,12 @@ mount_tmpfs_parseargs(int argc, char *ar
uid = a_uid(optarg);
uidset = 1;
break;
+   
+   case 'P':
+   if(strlcpy(template, optarg, MAXPATHLEN)
+ 

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Otto Moerbeek
On Fri, Sep 19, 2014 at 10:21:13PM +1000, Adam wrote:

> Patch for argument parsing below.
> 
> Diff taken in relation to Revision 3 of the aforementioned patch.
> 
> I do not understand why this should be a separate diff for this, as it
> is essentially part of the functionality itself.
> 
> Let me know if you want it all together.

Well, this is not what I meant.

I meant: if you want to restructure, do not mix that with the
additiont of functionality. 

-Otto

> 
> 
> 
> --- mount_tmpfs.c Fri Sep 19 12:04:50 2014
> +++ mount_tmpfs.c Fri Sep 19 12:14:21 2014
> @@ -88,7 +88,7 @@
>  void
>  mount_tmpfs_parseargs(int argc, char *argv[],
>   struct tmpfs_args *args, int *mntflags,
> - char *canon_dev, char *canon_dir)
> + char *template, char *canon_dir)
>  {
>   int gidset, modeset, uidset; /* Ought to be 'bool'. */
>   int ch;
> @@ -110,7 +110,7 @@
>   modeset = 0; mode = 0;
>  
>   optind = optreset = 1;
> - while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
> + while ((ch = getopt(argc, argv, "g:m:n:o:s:u:P:")) != -1 ) {
>   switch (ch) {
>   case 'g':
>   gid = a_gid(optarg);
> @@ -145,6 +145,12 @@
>   uid = a_uid(optarg);
>   uidset = 1;
>   break;
> + 
> + case 'P':
> + if(strlcpy(template, optarg, MAXPATHLEN)
> + >= MAXPATHLEN)
> + errx(1, "template path %s too long",
> optarg);
> + break;
>  
>   case '?':
>   default:
> @@ -157,7 +163,6 @@
>   if (argc != 2)
>   usage();
>  
> - strlcpy(canon_dev, argv[0], MAXPATHLEN);
>   pathadj(argv[1], canon_dir);
>  
>   if (stat(canon_dir, &sb) == -1)
> @@ -176,7 +181,7 @@
>   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);
> + "   [-u user] [-P template] tmpfs mount_point\n",
> __progname); exit(1);
>  }
>  
> @@ -186,12 +191,12 @@
>  mount_tmpfs(int argc, char *argv[])
>  {
>   struct tmpfs_args args;
> - char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
> + char canon_dir[MAXPATHLEN] = {'\0'};
>   char template[MAXPATHLEN] = {'\0'};
>   int mntflags, rdonly;
>  
>   mount_tmpfs_parseargs(argc, argv, &args, &mntflags,
> - canon_dev, canon_dir);
> + template, canon_dir);
>  
>   rdonly = mntflags & MNT_RDONLY;
>  



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
Patch for argument parsing below.

Diff taken in relation to Revision 3 of the aforementioned patch.

I do not understand why this should be a separate diff for this, as it
is essentially part of the functionality itself.

Let me know if you want it all together.



--- mount_tmpfs.c   Fri Sep 19 12:04:50 2014
+++ mount_tmpfs.c   Fri Sep 19 12:14:21 2014
@@ -88,7 +88,7 @@
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+   char *template, char *canon_dir)
 {
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
@@ -110,7 +110,7 @@
modeset = 0; mode = 0;
 
optind = optreset = 1;
-   while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
+   while ((ch = getopt(argc, argv, "g:m:n:o:s:u:P:")) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -145,6 +145,12 @@
uid = a_uid(optarg);
uidset = 1;
break;
+   
+   case 'P':
+   if(strlcpy(template, optarg, MAXPATHLEN)
+   >= MAXPATHLEN)
+   errx(1, "template path %s too long",
optarg);
+   break;
 
case '?':
default:
@@ -157,7 +163,6 @@
if (argc != 2)
usage();
 
-   strlcpy(canon_dev, argv[0], MAXPATHLEN);
pathadj(argv[1], canon_dir);
 
if (stat(canon_dir, &sb) == -1)
@@ -176,7 +181,7 @@
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);
+   "   [-u user] [-P template] tmpfs mount_point\n",
__progname); exit(1);
 }
 
@@ -186,12 +191,12 @@
 mount_tmpfs(int argc, char *argv[])
 {
struct tmpfs_args args;
-   char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
+   char canon_dir[MAXPATHLEN] = {'\0'};
char template[MAXPATHLEN] = {'\0'};
int mntflags, rdonly;
 
mount_tmpfs_parseargs(argc, argv, &args, &mntflags,
-   canon_dev, canon_dir);
+   template, canon_dir);
 
rdonly = mntflags & MNT_RDONLY;
 



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
Revision 3:
 - if (template) -> if (template[0] != '\0')

(From Revision 2):

1. Added code to allow updating MNT_RDONLY status with M.
   If MNT_WANTRDWR is specified as well, MNT_RDONLY is unset.

2. Added check for strlcpy() call.

3. If getenv() is deemed "unsafe" to call, just use the
   default "/tmp" path. If a warning should be shown here, let me
   know and I will change this to include one.




Index: sbin/mount_tmpfs/mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -r1.4 mount_tmpfs.c
--- sbin/mount_tmpfs/mount_tmpfs.c  21 Jan 2014 21:58:27 -  1.4
+++ sbin/mount_tmpfs/mount_tmpfs.c  19 Sep 2014 11:58:34 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,6 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
+
+#include "pathnames.h"
 
 #include "mount_tmpfs.h"
 
@@ -74,6 +78,13 @@ static void  pathadj(const char *, char *
 
 /* - */
 
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
@@ -176,13 +187,33 @@ mount_tmpfs(int argc, char *argv[])
 {
struct tmpfs_args args;
char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
-   int mntflags;
+   char template[MAXPATHLEN] = {'\0'};
+   int mntflags, rdonly;
 
mount_tmpfs_parseargs(argc, argv, &args, &mntflags,
canon_dev, canon_dir);
 
+   rdonly = mntflags & MNT_RDONLY;
+
+   if(template[0] != '\0')
+   mntflags &= ~MNT_RDONLY;
+
if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) == -1)
err(EXIT_FAILURE, "tmpfs on %s", canon_dir);
+   
+   if(template[0] != '\0') {
+   tcopy(template, canon_dir, &args);
+
+   if (rdonly) {
+   mntflags |= MNT_RDONLY | MNT_UPDATE;
+   if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) < 0) 
{
+   warn("%s: mount (update, rdonly)", canon_dir);
+   if (unmount(canon_dir, 0) != 0)
+   warn("unmount %s", canon_dir);
+   exit(1);
+   }
+   }
+   }
 
return EXIT_SUCCESS;
 }
@@ -249,4 +280,139 @@ pathadj(const char *input, char *adjuste
warnx("\"%s\" is a non-resolved or relative path.", input);
warnx("using \"%s\" instead.", adjusted);
}
+}
+
+/* Code copied from sbin/newfs/newfs.c to copy a template. */
+
+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
+isdir(const char *path)
+{
+   struct stat st;
+
+   if (stat(path, &st) != 0)
+   err(1, "cannot stat %s", path);
+   if (!S_ISDIR(st.st_mode) && !S_ISBLK(st.st_mode))
+   errx(1, "%s: not a dir or a block device", path);
+   return (S_ISDIR(st.st_mode));
+}
+
+static void
+tcopy(char *src, char *dst, struct tmpfs_args *args)
+{
+   int ret, dir, created = 0;
+   struct ufs_args m

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
On Fri, 19 Sep 2014 10:58:59 +0200
Henning Brauer  wrote:

> HUH?
> 
> Doug is entirely right. src is user controlled and can be larger than
> mountpoint. In that case, we want to bail and whine at the user
> instead of silently truncating and going on.
> 

Thanks for clarifying that; I thought he was talking about simply
checking it before calling strlcpy().



Updated diff follows:

1. Added code to allow updating MNT_RDONLY status with M.
   If MNT_WANTRDWR is specified as well, MNT_RDONLY is unset.

2. Added check for strlcpy() call.

3. If getenv() is deemed "unsafe" to call, just use the
   default "/tmp" path. If a warning should be shown here, let me
   know and I will change this to include one.




Index: sbin/mount_tmpfs/mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mount_tmpfs.c
--- sbin/mount_tmpfs/mount_tmpfs.c  21 Jan 2014 21:58:27 -  1.4
+++ sbin/mount_tmpfs/mount_tmpfs.c  19 Sep 2014 10:06:31 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,6 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
+
+#include "pathnames.h"
 
 #include "mount_tmpfs.h"
 
@@ -74,6 +78,13 @@ static void  pathadj(const char *, char *
 
 /* - */
 
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
@@ -176,13 +187,33 @@ mount_tmpfs(int argc, char *argv[])
 {
struct tmpfs_args args;
char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
-   int mntflags;
+   char template[MAXPATHLEN] = {'\0'};
+   int mntflags, rdonly;
 
mount_tmpfs_parseargs(argc, argv, &args, &mntflags,
canon_dev, canon_dir);
 
+   rdonly = mntflags & MNT_RDONLY;
+
+   if(template)
+   mntflags &= ~MNT_RDONLY;
+
if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) == -1)
err(EXIT_FAILURE, "tmpfs on %s", canon_dir);
+   
+   if(template[0] != '\0') {
+   tcopy(template, canon_dir, &args);
+
+   if (rdonly) {
+   mntflags |= MNT_RDONLY | MNT_UPDATE;
+   if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) < 0) 
{
+   warn("%s: mount (update, rdonly)", canon_dir);
+   if (unmount(canon_dir, 0) != 0)
+   warn("unmount %s", canon_dir);
+   exit(1);
+   }
+   }
+   }
 
return EXIT_SUCCESS;
 }
@@ -249,4 +280,139 @@ pathadj(const char *input, char *adjuste
warnx("\"%s\" is a non-resolved or relative path.", input);
warnx("using \"%s\" instead.", adjusted);
}
+}
+
+/* Code copied from sbin/newfs/newfs.c to copy a template. */
+
+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
+isdir(const char *path)
+{
+   struct stat st;
+
+   if (stat(path, &st) != 0)
+   err(1, "cannot stat %s", pa

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
Documentation update:




Index: mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -r1.3 mount_tmpfs.8
--- mount_tmpfs.8   5 Feb 2014 15:32:26 -   1.3
+++ mount_tmpfs.8   19 Sep 2014 11:30:33 -
@@ -43,6 +43,7 @@
 .Op Fl o Ar options
 .Op Fl s Ar size
 .Op Fl u Ar user
+.Op Fl P Ar template
 .Ar tmpfs
 .Ar mount_point
 .Sh DESCRIPTION
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl P Ar template
+If
+.Ar template
+is a directory, populate the created tmpfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created tmpfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl P Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Henning Brauer
* Adam  [2014-09-19 10:37]:
> On Fri, 19 Sep 2014 00:14:47 -0700
> Doug Hogan  wrote:
> > On Thu, Sep 18, 2014 at 09:28:41AM +0100, bytevolc...@safe-mail.net
> > wrote:
> > > + strlcpy(mountpoint, src, sizeof(mountpoint));
> > 
> > The strlcpy call should check for truncation.  There's no guarantee
> > that src is less than sizeof(mountpoint) since src = template =
> > optarg.
> 
> According to man strlcpy(3):
> 
> "strlcpy() copies up to dstsize - 1 characters from the string src to
> dst, NUL-terminating the result if dstsize is not 0."
> 
> Thus, such a check here would be redundant.

HUH?

Doug is entirely right. src is user controlled and can be larger than
mountpoint. In that case, we want to bail and whine at the user
instead of silently truncating and going on.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
On Fri, 19 Sep 2014 00:14:47 -0700
Doug Hogan  wrote:

> On Thu, Sep 18, 2014 at 09:28:41AM +0100, bytevolc...@safe-mail.net
> wrote:
> > Revised diffbelow:
> 
> You are inheriting some bugs in here by reusing newfs code. :)
> 
> I agree with the comments from others.  Additional comments:
> 
> >  #include 
> > +#include 
> ...
> > +   args.ta_root_mode = modeset ? mode : sb.st_mode;
> > +
> > +   int newflags = mntflags;
> ...
> > +   if(template) newflags &= ~MNT_RDONLY;
> ...
> > +   if(template) {
> 
> KNF
> 
> > +#include "pathnames.h"
> 
> This was too much copy/paste from newfs.  You aren't using any of
> the definitions in here.

Sorted.

> 
> > +   if (execv(cmd, argv) != 0)
> > +   err(1, "%s", cmd);
> 
> execv() does not return when successful.  On error, it returns -1.

Hence the need for the code. The check appears to be more of a
semantics thin to me. Yes, it will not return if successful, but if it
is not successful, there should still be some code to handle this case.

At least here, it looks like a typical error handling "if" statement
rather than just "Do this, but act like there's an error whether or
not we are successful."

> 
> > +   strlcpy(mountpoint, src, sizeof(mountpoint));
> 
> The strlcpy call should check for truncation.  There's no guarantee
> that src is less than sizeof(mountpoint) since src = template =
> optarg.

According to man strlcpy(3):

"strlcpy() copies up to dstsize - 1 characters from the string src to
dst, NUL-terminating the result if dstsize is not 0."

Thus, such a check here would be redundant.

> > +   tmp = getenv("TMPDIR");
> > +   if (tmp == NULL || *tmp == '\0')
> > +   tmp = _PATH_TMP;
> 
> It's being used with mkdtemp(), but I think you should still check
> issetugid() before trusting getenv().

Looking into this now.

> 
> + if (mntflags & MNT_RDONLY) {
> + mntflags |= MNT_UPDATE;
> + if (mount(MOUNT_TMPFS, canon_dir, mntflags,
> &args) < 0) {
> 
> This doesn't look right.  /usr/src/sys/tmpfs/tmpfs_vfsops.c doesn't
> support MNT_UPDATE in tmpfs_mount().

I have added some code to allow updating of MNT_RDONLY, and it also
responds to MNT_WANTRDWR by unsetting MNT_RDONLY.

As I see from tmpfs_vnops.c as well, the MNT_RDONLY flag is used here
to prevent writing to files. There also appears to be a bug here: it
appears to be possible to delete files from a "read-only" tmpfs file
system.

> 
> I don't see a reason for canon_dev to exist.  It gets written to but
> isn't used anywhere else.

I am going to fix this in another diff since Otto suggested parsing
changes should be kept separate from functionality.



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Adam
Revision 2:


1. Fixed up an issue caused by bad diffing technique.

2. Only functionality changes included here; parsing changes to follow next
   as per Otto's instruction.

3. Documentation changes will follow as well.

4. I have received some more feedback from Doug Hogan regarding newfs bugs,
   and some more changes are coming up.

5. Modified pathnames.h a little. What is the consensus on defining things
   like _PATH_MNT here? I am finding it difficult to justify having an
   entire header file just for one #define, but I am unsure what OpenBSD
   considers acceptable. Input here appreciated.



Index: mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mount_tmpfs.c
--- mount_tmpfs.c   21 Jan 2014 21:58:27 -  1.4
+++ mount_tmpfs.c   19 Sep 2014 05:23:16 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,6 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
+
+#include "pathnames.h"
 
 #include "mount_tmpfs.h"
 
@@ -74,6 +78,13 @@ static void  pathadj(const char *, char *
 
 /* - */
 
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
@@ -176,13 +187,31 @@ mount_tmpfs(int argc, char *argv[])
 {
struct tmpfs_args args;
char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
+   char template[MAXPATHLEN] = {'\0'};
int mntflags;
 
mount_tmpfs_parseargs(argc, argv, &args, &mntflags,
canon_dev, canon_dir);
 
+   int rdonly = mntflags & MNT_RDONLY;
+   if(template) mntflags &= ~MNT_RDONLY;
+
if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) == -1)
err(EXIT_FAILURE, "tmpfs on %s", canon_dir);
+   
+   if(template[0] != '\0') {
+   tcopy(template, canon_dir, &args);
+
+   if (rdonly) {
+   mntflags |= MNT_RDONLY | MNT_UPDATE;
+   if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) < 0) 
{
+   warn("%s: mount (update, rdonly)", canon_dir);
+   if (unmount(canon_dir, 0) != 0)
+   warn("unmount %s", canon_dir);
+   exit(1);
+   }
+   }
+   }
 
return EXIT_SUCCESS;
 }
@@ -249,4 +278,133 @@ pathadj(const char *input, char *adjuste
warnx("\"%s\" is a non-resolved or relative path.", input);
warnx("using \"%s\" instead.", adjusted);
}
+}
+
+/* Code grafted from sbin/newfs/newfs.c to copy a template. */
+
+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
+isdir(const char *path)
+{
+   struct stat st;
+
+   if (stat(path, &st) != 0)
+   err(1, "cannot stat %s", path);
+   if (!S_ISDIR(st.st_mode) && !S_ISBLK(st.st_mode))
+   errx(1, "%s: not a dir or a block device", path);
+   return (S_ISDIR(st.st_mode));
+}
+
+static void
+tcopy(char *s

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-19 Thread Doug Hogan
On Thu, Sep 18, 2014 at 09:28:41AM +0100, bytevolc...@safe-mail.net wrote:
> Revised diffbelow:

You are inheriting some bugs in here by reusing newfs code. :)

I agree with the comments from others.  Additional comments:

>  #include 
> +#include 
...
> + args.ta_root_mode = modeset ? mode : sb.st_mode;
> +
> + int newflags = mntflags;
...
> + if(template) newflags &= ~MNT_RDONLY;
...
> + if(template) {

KNF

> +#include "pathnames.h"

This was too much copy/paste from newfs.  You aren't using any of
the definitions in here.

> + if (execv(cmd, argv) != 0)
> + err(1, "%s", cmd);

execv() does not return when successful.  On error, it returns -1.

> + strlcpy(mountpoint, src, sizeof(mountpoint));

The strlcpy call should check for truncation.  There's no guarantee
that src is less than sizeof(mountpoint) since src = template = optarg.

> + tmp = getenv("TMPDIR");
> + if (tmp == NULL || *tmp == '\0')
> + tmp = _PATH_TMP;

It's being used with mkdtemp(), but I think you should still check
issetugid() before trusting getenv().

+   if (mntflags & MNT_RDONLY) {
+   mntflags |= MNT_UPDATE;
+   if (mount(MOUNT_TMPFS, canon_dir, mntflags, &args) < 0) 
{

This doesn't look right.  /usr/src/sys/tmpfs/tmpfs_vfsops.c doesn't
support MNT_UPDATE in tmpfs_mount().


I don't see a reason for canon_dev to exist.  It gets written to but isn't
used anywhere else.



Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-18 Thread Alexander Hall


On September 18, 2014 9:40:44 AM CEST, bytevolc...@safe-mail.net wrote:
>This patch adds an option "-t template" to mount_tmpfs, which
>populates the new tmpfs volume with a directory
>immediately after creation.
>
>Man page update included for explanation.
>
>Much of the code was grafted from newfs
>which implements this for mount_mfs ("-P" option).
>
>Suggestions, fixes, criticism, etc. welcome.
>
>Index: mount_tmpfs.8
>===
>RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
>retrieving revision 1.3
>diff -u -p -u -r1.3 mount_tmpfs.8
>--- mount_tmpfs.8  5 Feb 2014 15:32:26 -   1.3
>+++ mount_tmpfs.8  17 Sep 2014 21:37:43 -
>@@ -42,6 +42,7 @@
> .Op Fl n Ar nodes
> .Op Fl o Ar options
> .Op Fl s Ar size
>+.Op Fl t Ar template
> .Op Fl u Ar user
> .Ar tmpfs
> .Ar mount_point
>@@ -86,10 +87,28 @@ If zero is given (the default), the avai
> main memory and swap space) will be used.
> Note that four megabytes are always reserved for the system and cannot
> be assigned to the file system.
>+.It Fl t Ar template
>+If
>+.Ar template
>+is a directory, populate the created mfs file system with the
>+contents of the directory.
>+If
>+.Ar template
>+is a block device, populate the created mfs file system with the
>+contents of the FFS file system contained on the device.
> .It Fl u Ar user
> Specifies the user name or UID of the root inode of the file system.
> Defaults to the mount point's UID.
> .El
>+.Pp
>+When the
>+.Fl t Ar template
>+option is used, permissions are always copied from the template.
>+The
>+.Fl u , Fl g , 
>+and
>+.Fl m
>+options only affect the root inode of the file system.
> .Pp
> Every option that accepts a numerical value as its argument can take a
> trailing
>Index: mount_tmpfs.c
>===
>RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
>retrieving revision 1.4
>diff -u -p -u -r1.4 mount_tmpfs.c
>--- mount_tmpfs.c  21 Jan 2014 21:58:27 -  1.4
>+++ mount_tmpfs.c  17 Sep 2014 21:37:43 -
>@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
> #include 
> #include 
> #include 
>+#include 
> 
> #include 
> #include 
>@@ -51,8 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
> #include 
> #include 
> #include 
>+#include 
> 
>-#include "mount_tmpfs.h"
>+#include "pathnames.h"
> 
>/*
>-
>*/
> 
>@@ -74,11 +76,34 @@ static voidpathadj(const char *, char *
> 
>/*
>-
>*/
> 
>-void
>-mount_tmpfs_parseargs(int argc, char *argv[],
>-  struct tmpfs_args *args, int *mntflags,
>-  char *canon_dev, char *canon_dir)
>+static int do_exec(const char *, const char *, char *const[]);
>+static int isdir(const char *);
>+static void tcopy(char *, char *, struct tmpfs_args *);
>+static int gettmpmnt(char *, size_t);
>+
>+/*
>-
>*/
>+
>+static void
>+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);

Aren't you missing something relevant here...? :-)

/Alexander

>+  exit(1);
>+}
>+
>+/*
>-
>*/
>+
>+int
>+mount_tmpfs(int argc, char *argv[])
> {
>+  struct tmpfs_args args = {0};
>+  char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
>+  int mntflags = 0;
>+  
>+  char *template = NULL;
>+
>   int gidset, modeset, uidset; /* Ought to be 'bool'. */
>   int ch;
>   gid_t gid;
>@@ -88,18 +113,16 @@ mount_tmpfs_parseargs(int argc, char *ar
>   struct stat sb;
> 
>   /* Set default values for mount point arguments. */
>-  memset(args, 0, sizeof(*args));
>-  args->ta_version = TMPFS_ARGS_VERSION;
>-  args->ta_size_max = 0;
>-  args->ta_nodes_max = 0;
>-  *mntflags = 0;
>+  args.ta_version = TMPFS_ARGS_VERSION;
>+  args.ta_size_max = 0;
>+  args.ta_nodes_max = 0;
> 
>   gidset = 0; gid = 0;
>   uidset = 0; uid = 0;
>   modeset = 0; mode = 0;
> 
>   optind = optreset = 1;
>-  while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
>+  while ((ch = getopt(argc, argv, "g:m:n:o:s:t:u:")) != -1 ) {
>   switch (ch) {
>   case 'g':
>   gid = a_gid(optarg);
>@@ -116,18 +139,22 @@ mount_tmpfs_parseargs(int argc, char *ar
>   if (scan_scaled(optarg, &tmpnumber) == -1)
>   err(EXIT_FAILURE, "failed to parse nodes `%s'",
>   optarg);
>-  args->ta_nodes_max = tmpnumber;
>+  args.ta_nodes_max = tmpnumber;
>   break;
> 
> 

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-18 Thread bytevolcano
>In the mount_tmpfs.8 diff, you talk about mfs... :)
>
>Landry

Serves me right for coding while I'm sick.

Revised diffbelow:




Index: mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -r1.3 mount_tmpfs.8
--- mount_tmpfs.8   5 Feb 2014 15:32:26 -   1.3
+++ mount_tmpfs.8   18 Sep 2014 08:22:42 -
@@ -42,6 +42,7 @@
 .Op Fl n Ar nodes
 .Op Fl o Ar options
 .Op Fl s Ar size
+.Op Fl t Ar template
 .Op Fl u Ar user
 .Ar tmpfs
 .Ar mount_point
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl t Ar template
+If
+.Ar template
+is a directory, populate the created tmpfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created tmpfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl t Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing
Index: mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -r1.4 mount_tmpfs.c
--- mount_tmpfs.c   21 Jan 2014 21:58:27 -  1.4
+++ mount_tmpfs.c   18 Sep 2014 08:22:42 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,8 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
-#include "mount_tmpfs.h"
+#include "pathnames.h"
 
 /* - */
 
@@ -74,11 +76,34 @@ static void pathadj(const char *, char *
 
 /* - */
 
-void
-mount_tmpfs_parseargs(int argc, char *argv[],
-   struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
+static void
+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);
+   exit(1);
+}
+
+/* - */
+
+int
+mount_tmpfs(int argc, char *argv[])
 {
+   struct tmpfs_args args = {0};
+   char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
+   int mntflags = 0;
+   
+   char *template = NULL;
+
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
gid_t gid;
@@ -88,18 +113,16 @@ mount_tmpfs_parseargs(int argc, char *ar
struct stat sb;
 
/* Set default values for mount point arguments. */
-   memset(args, 0, sizeof(*args));
-   args->ta_version = TMPFS_ARGS_VERSION;
-   args->ta_size_max = 0;
-   args->ta_nodes_max = 0;
-   *mntflags = 0;
+   args.ta_version = TMPFS_ARGS_VERSION;
+   args.ta_size_max = 0;
+   args.ta_nodes_max = 0;
 
gidset = 0; gid = 0;
uidset = 0; uid = 0;
modeset = 0; mode = 0;
 
optind = optreset = 1;
-   while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
+   while ((ch = getopt(argc, argv, "g:m:n:o:s:t:u:")) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -116,18 +139,22 @@ mount_tmpfs_parseargs(int argc, char *ar
if (scan_scaled(optarg, &tmpnumber) == -1)
err(EXIT_FAILURE, "failed to parse nodes `%s'",
optarg);
-   args->ta_nodes_max = tmpnumber;
+   args.ta_nodes_max = tmpnumber;
break;
 
case 'o':
-   getmntopts(optarg, mopts, mntflags);
+   getmntopts(optarg, mopts, &mntflags);
break;
 
case 's':
if (scan_scaled(optarg, &tmpnumber) == -1)
err(EXIT_FAILURE, "failed to parse size `%s'",
optarg);
-   args->ta_size_max = tmpnumber;
+  

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-18 Thread Otto Moerbeek
On Thu, Sep 18, 2014 at 08:40:44AM +0100, bytevolc...@safe-mail.net wrote:

> This patch adds an option "-t template" to mount_tmpfs, which
> populates the new tmpfs volume with a directory
> immediately after creation.
> 
> Man page update included for explanation.
> 
> Much of the code was grafted from newfs
> which implements this for mount_mfs ("-P" option).
> 
> Suggestions, fixes, criticism, etc. welcome.

- Please do not mix the argument parsing changes with the new
  functionality changes. Post separate diffs for that, although I do not
  see the point of removing mount_tmpfs_parseargs(). 
- Why use a different flag compared to mount_mfs?
- What's pathnames.h doing here?

-Otto

> 
> Index: mount_tmpfs.8
> ===
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
> retrieving revision 1.3
> diff -u -p -u -r1.3 mount_tmpfs.8
> --- mount_tmpfs.8 5 Feb 2014 15:32:26 -   1.3
> +++ mount_tmpfs.8 17 Sep 2014 21:37:43 -
> @@ -42,6 +42,7 @@
>  .Op Fl n Ar nodes
>  .Op Fl o Ar options
>  .Op Fl s Ar size
> +.Op Fl t Ar template
>  .Op Fl u Ar user
>  .Ar tmpfs
>  .Ar mount_point
> @@ -86,10 +87,28 @@ If zero is given (the default), the avai
>  main memory and swap space) will be used.
>  Note that four megabytes are always reserved for the system and cannot
>  be assigned to the file system.
> +.It Fl t Ar template
> +If
> +.Ar template
> +is a directory, populate the created mfs file system with the
> +contents of the directory.
> +If
> +.Ar template
> +is a block device, populate the created mfs file system with the
> +contents of the FFS file system contained on the device.
>  .It Fl u Ar user
>  Specifies the user name or UID of the root inode of the file system.
>  Defaults to the mount point's UID.
>  .El
> +.Pp
> +When the
> +.Fl t Ar template
> +option is used, permissions are always copied from the template.
> +The
> +.Fl u , Fl g , 
> +and
> +.Fl m
> +options only affect the root inode of the file system.
>  .Pp
>  Every option that accepts a numerical value as its argument can take a
>  trailing
> Index: mount_tmpfs.c
> ===
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
> retrieving revision 1.4
> diff -u -p -u -r1.4 mount_tmpfs.c
> --- mount_tmpfs.c 21 Jan 2014 21:58:27 -  1.4
> +++ mount_tmpfs.c 17 Sep 2014 21:37:43 -
> @@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -51,8 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
>  #include 
>  #include 
>  #include 
> +#include 
>  
> -#include "mount_tmpfs.h"
> +#include "pathnames.h"
>  
>  /* - */
>  
> @@ -74,11 +76,34 @@ static void   pathadj(const char *, char *
>  
>  /* - */
>  
> -void
> -mount_tmpfs_parseargs(int argc, char *argv[],
> - struct tmpfs_args *args, int *mntflags,
> - char *canon_dev, char *canon_dir)
> +static int do_exec(const char *, const char *, char *const[]);
> +static int isdir(const char *);
> +static void tcopy(char *, char *, struct tmpfs_args *);
> +static int gettmpmnt(char *, size_t);
> +
> +/* - */
> +
> +static void
> +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);
> + exit(1);
> +}
> +
> +/* - */
> +
> +int
> +mount_tmpfs(int argc, char *argv[])
>  {
> + struct tmpfs_args args = {0};
> + char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
> + int mntflags = 0;
> + 
> + char *template = NULL;
> +
>   int gidset, modeset, uidset; /* Ought to be 'bool'. */
>   int ch;
>   gid_t gid;
> @@ -88,18 +113,16 @@ mount_tmpfs_parseargs(int argc, char *ar
>   struct stat sb;
>  
>   /* Set default values for mount point arguments. */
> - memset(args, 0, sizeof(*args));
> - args->ta_version = TMPFS_ARGS_VERSION;
> - args->ta_size_max = 0;
> - args->ta_nodes_max = 0;
> - *mntflags = 0;
> + args.ta_version = TMPFS_ARGS_VERSION;
> + args.ta_size_max = 0;
> + args.ta_nodes_max = 0;
>  
>   gidset = 0; gid = 0;
>   uidset = 0; uid = 0;
>   modeset = 0; mode = 0;
>  
>   optind = optreset = 1;
> - while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
> + while ((ch = getopt(argc, argv, "g:m:n:o:s:t:u:")) != -1 ) {
>   switch (ch) {
>   case 'g':
>   gid = a_gid(optarg);
> @@ -116,18 +139,22 @@ mount_tmpfs_parseargs(int argc, char *ar
>  

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-18 Thread Landry Breuil
On Thu, Sep 18, 2014 at 08:40:44AM +0100, bytevolc...@safe-mail.net wrote:
> This patch adds an option "-t template" to mount_tmpfs, which
> populates the new tmpfs volume with a directory
> immediately after creation.
> 
> Man page update included for explanation.
> 
> Much of the code was grafted from newfs
> which implements this for mount_mfs ("-P" option).
> 
> Suggestions, fixes, criticism, etc. welcome.

In the mount_tmpfs.8 diff, you talk about mfs... :)

Landry



[PATCH] Option for mount_tmpfs to populate the volume after creation.

2014-09-18 Thread bytevolcano
This patch adds an option "-t template" to mount_tmpfs, which
populates the new tmpfs volume with a directory
immediately after creation.

Man page update included for explanation.

Much of the code was grafted from newfs
which implements this for mount_mfs ("-P" option).

Suggestions, fixes, criticism, etc. welcome.

Index: mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -u -r1.3 mount_tmpfs.8
--- mount_tmpfs.8   5 Feb 2014 15:32:26 -   1.3
+++ mount_tmpfs.8   17 Sep 2014 21:37:43 -
@@ -42,6 +42,7 @@
 .Op Fl n Ar nodes
 .Op Fl o Ar options
 .Op Fl s Ar size
+.Op Fl t Ar template
 .Op Fl u Ar user
 .Ar tmpfs
 .Ar mount_point
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl t Ar template
+If
+.Ar template
+is a directory, populate the created mfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created mfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl t Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing
Index: mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mount_tmpfs.c
--- mount_tmpfs.c   21 Jan 2014 21:58:27 -  1.4
+++ mount_tmpfs.c   17 Sep 2014 21:37:43 -
@@ -39,6 +39,7 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -51,8 +52,9 @@ __RCSID("$NetBSD: mount_tmpfs.c,v 1.24 2
 #include 
 #include 
 #include 
+#include 
 
-#include "mount_tmpfs.h"
+#include "pathnames.h"
 
 /* - */
 
@@ -74,11 +76,34 @@ static void pathadj(const char *, char *
 
 /* - */
 
-void
-mount_tmpfs_parseargs(int argc, char *argv[],
-   struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
+static void
+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);
+   exit(1);
+}
+
+/* - */
+
+int
+mount_tmpfs(int argc, char *argv[])
 {
+   struct tmpfs_args args = {0};
+   char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
+   int mntflags = 0;
+   
+   char *template = NULL;
+
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
gid_t gid;
@@ -88,18 +113,16 @@ mount_tmpfs_parseargs(int argc, char *ar
struct stat sb;
 
/* Set default values for mount point arguments. */
-   memset(args, 0, sizeof(*args));
-   args->ta_version = TMPFS_ARGS_VERSION;
-   args->ta_size_max = 0;
-   args->ta_nodes_max = 0;
-   *mntflags = 0;
+   args.ta_version = TMPFS_ARGS_VERSION;
+   args.ta_size_max = 0;
+   args.ta_nodes_max = 0;
 
gidset = 0; gid = 0;
uidset = 0; uid = 0;
modeset = 0; mode = 0;
 
optind = optreset = 1;
-   while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
+   while ((ch = getopt(argc, argv, "g:m:n:o:s:t:u:")) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -116,18 +139,22 @@ mount_tmpfs_parseargs(int argc, char *ar
if (scan_scaled(optarg, &tmpnumber) == -1)
err(EXIT_FAILURE, "failed to parse nodes `%s'",
optarg);
-   args->ta_nodes_max = tmpnumber;
+   args.ta_nodes_max = tmpnumber;
break;
 
case 'o':
-   getmntopts(optarg, mopts, mntflags);
+   getmntopts(optarg, mopts, &mntflags);
break;
 
case 's':
if (scan_scaled(optarg, &tmpnumber