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 (obr...@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);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
