Date: Mon, 12 Jan 2009 13:15:07 -0800 From: "David O'Brien" <obrien@freebsd.org> To: Christoph Mallon <christoph.mallon@gmx.de> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r186504 - head/sbin/mount Message-ID: <20090112211507.GA90878@dragon.NUXI.org> In-Reply-To: <496B10F9.20201@gmx.de> References: <200812262254.mBQMsrbR052676@svn.freebsd.org> <4960FA9A.1090509@gmx.de> <20090111041543.GB17602@dragon.NUXI.org> <4969A626.6070908@gmx.de> <20090112082510.GA69194@dragon.NUXI.org> <496B10F9.20201@gmx.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090112211507.GA90878>