From owner-svn-src-all@FreeBSD.ORG Mon Jan 12 21:15:08 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 73899106564A; Mon, 12 Jan 2009 21:15:08 +0000 (UTC) (envelope-from obrien@NUXI.org) Received: from dragon.nuxi.org (trang.nuxi.org [74.95.12.85]) by mx1.freebsd.org (Postfix) with ESMTP id 393D98FC16; Mon, 12 Jan 2009 21:15:07 +0000 (UTC) (envelope-from obrien@NUXI.org) Received: from dragon.nuxi.org (obrien@localhost [127.0.0.1]) by dragon.nuxi.org (8.14.2/8.14.2) with ESMTP id n0CLF71v093990; Mon, 12 Jan 2009 13:15:07 -0800 (PST) (envelope-from obrien@dragon.nuxi.org) Received: (from obrien@localhost) by dragon.nuxi.org (8.14.2/8.14.2/Submit) id n0CLF70v093989; Mon, 12 Jan 2009 13:15:07 -0800 (PST) (envelope-from obrien) Date: Mon, 12 Jan 2009 13:15:07 -0800 From: "David O'Brien" To: Christoph Mallon Message-ID: <20090112211507.GA90878@dragon.NUXI.org> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=unknown-8bit Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <496B10F9.20201@gmx.de> X-Operating-System: FreeBSD 8.0-CURRENT User-Agent: Mutt/1.5.16 (2007-06-09) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r186504 - head/sbin/mount X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: obrien@freebsd.org List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Jan 2009 21:15:09 -0000 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);