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>