On Wed, 18 Oct 2017 15:03:21 +0200
Martin Pieuchot <m...@openbsd.org> wrote:
> On 18/10/17(Wed) 12:51, Helg Bredow wrote:
> > On Wed, 18 Oct 2017 10:04:07 +0200
> > Martin Pieuchot <m...@openbsd.org> wrote:
> >
> > > On 17/10/17(Tue) 15:30, Helg Bredow wrote:
> > > > If you execute "fuse-zip -V" it prints the version and then dumps core.
> > > > This is because fuse-zip does not initialise the mount point pointer to
> > > > NULL. This patch ensures that it's always initialised to NULL.
> > >
> > > It's hard to understand your fix if you don't explain what "dumps core".
> > >
> > > I had to install the package and look at the trace myself. You could
> > > save me these tasks by either posting the backtrace, saying that free(3)
> > > is call on an uninitialized memory or both.
> > >
> > > That said, I'd suggest different fix. Initializing `mp' in fuse_setup()
> > > is very fragile. Instead I'd declare a local variable and don't use
> > > `mp' at all in these function.
> > > In case of sucsses, just before returning the "struct fuse" pointer I'd
> > > assign *mp, if not NULL, to the local variable.
> > >
> > > By the way, what does "mp" stand for? I find the name confusing.
> > >
> >
> > Sorry, I've been looking at this for so long I assumed the diff was
> > self-explanatory. Thanks for your patience. I like your suggestion and have
> > modified the patch accordingly. A NULL test before the assignment is
> > superfluous since the code won't be reached if dir is NULL.
>
> But what if mp is NULL?
Good point; I hadn't considered that. I've added the check.
>
> > `mp' is the directory where the fuse file system will be mounted (mount
> > point). I've named the new variable dir to be consistent with mount(2).
>
> Nice.
Index: fuse.c
===================================================================
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.29
diff -u -p -r1.29 fuse.c
--- fuse.c 21 Aug 2017 21:41:13 -0000 1.29
+++ fuse.c 18 Oct 2017 14:30:08 -0000
@@ -466,14 +466,16 @@ fuse_setup(int argc, char **argv, const
struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
struct fuse_chan *fc;
struct fuse *fuse;
+ char *dir;
int fg;
- if (fuse_parse_cmdline(&args, mp, mt, &fg))
+ dir = NULL;
+ if (fuse_parse_cmdline(&args, &dir, mt, &fg))
goto err;
fuse_daemonize(0);
- if ((fc = fuse_mount(*mp, NULL)) == NULL)
+ if ((fc = fuse_mount(dir, NULL)) == NULL)
goto err;
if ((fuse = fuse_new(fc, NULL, ops, size, data)) == NULL) {
@@ -481,9 +483,12 @@ fuse_setup(int argc, char **argv, const
goto err;
}
+ if (mp != NULL)
+ *mp = dir;
+
return (fuse);
err:
- free(*mp);
+ free(dir);
return (NULL);
}
