svn commit: r186504 - head/sbin/mount

[David O'Brien]

On Mon, Jan 12, 2009 at 10:44:25AM +0100, Christoph Mallon wrote:
> Alternatively I would have gladly reviewed your solution.

Feel free to review the below.


> Actually this version doesn't work either.
> tron# ./mount -a
> usage: mount [-t fstype] [-o options] target_fs mount_point
> tron# ./mount -d -a
> mount -t ufs -o rw -o update /dev/ad0s1a /
> mount -t ufs ��`X (濿��`X /dev/ad0s1f /data
> (Sorry for the garbage, it actually printed that. You can turn it into a 
> "clean" segfault by memset()ing mnt_argv just after its declaration)
 
> Your incorrect use of local variables and the resulting undefined behaviour 
> made the cat, who visits me on a roof behind the house sometimes, almost 
> fall from said roof, when he saw your commit: You expect the local variable 
> "struct cpa mnt_argv" still to have the same values after mountfs() was 
> left and reentered. Too bad you didn't at least follow the lead of what I 
> proposed.

Please stop with the attitude - not one of your patches has passed
the simple smoke test - "does it boot".

Yes I goofed.  I was unfortunately too lucky on my test AMD64
system - things just kept lining up perfectly for me.  I partially took
care of the reenterancy (I'm not sure you realized mountfs() is called
multiple times), but failed to ensure the lifetime of the pointer.

-- 
-- David  (obrien at FreeBSD.org)


Index: mount.c
===================================================================
--- mount.c	(revision 187107)
+++ mount.c	(working copy)
@@ -518,11 +518,10 @@ int
 mountfs(const char *vfstype, const char *spec, const char *name, int flags,
 	const char *options, const char *mntopts)
 {
-	struct cpa mnt_argv;
 	struct statfs sf;
 	int i, ret;
 	char *optbuf, execname[PATH_MAX], mntpath[PATH_MAX];
-	static int mnt_argv_inited;
+	static struct cpa mnt_argv;
 
 	/* resolve the mountpoint with realpath(3) */
 	(void)checkpath(name, mntpath);
@@ -557,10 +556,6 @@ mountfs(const char *vfstype, const char 
 	/* Construct the name of the appropriate mount command */
 	(void)snprintf(execname, sizeof(execname), "mount_%s", vfstype);
 
-	if (!mnt_argv_inited) {
-		mnt_argv_inited++;
-		mnt_argv.a = NULL;
-	}
 	mnt_argv.c = -1;
 	append_arg(&mnt_argv, execname);
 	mangle(optbuf, &mnt_argv);