On 25/10/17(Wed) 13:27, Helg Bredow wrote:
> I've included different minor patches below as one patch. I haven't split
> into separate patches since the changes are not complex and easy to audit.
>
> Here's what it does:
>
> Almost all functions in fuse.c do not check if the arguments are null. This
> patch adds null checks where appropriate.
>
> Some arguments are incorrectly flagged as unused. Delete "unused" if the
> argument is used in the function.
>
> The wrong version macro is used in dump_version() and is inconsistent with
> that used in fuse_version(). FUSE_USE_VERSION is intended to be defined by
> file system to specify which backward compatible version of libfuse they
> require.
>
> fuse_loop_mt() is not implemented so return -1 rather than success. If a file
> system tries to call it then it should be obvious that it's not going to work.
>
> fuse_main() declared redundant variables due to the lack of NULL checks
> before assignment. We now check for NULL so can safely pass NULL instead.
>
> Tested with a regression test that passes all NULL arguments to exported
> functions.
>
> Something to consider is that fuse_is_lib_option() is deprecated. Should we
> remove it from libfuse to stay strictly at version 26?
Testing for NULL is good. However returning -1 in fuse_loop_mt() and
changing the version number might break ports. So if these changes go
in, you should watch for regression on ports@ at least.
We can remove fuse_is_lib_option() if nothing in ports use it. A good
start to search for it is codesearch.debian.net. You can also ask
porters to do a grep on unpacked port tree.
Could you provide a diff including only the NULL check and the removal
of "unused" markers?
> Index: fuse.c
> ===================================================================
> RCS file: /cvs/src/lib/libfuse/fuse.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 fuse.c
> --- fuse.c 25 Oct 2017 09:29:46 -0000 1.31
> +++ fuse.c 25 Oct 2017 12:54:48 -0000
> @@ -71,6 +71,9 @@ fuse_loop(struct fuse *fuse)
> ssize_t n;
> int ret;
>
> + if (fuse == NULL)
> + return (-1);
> +
> fuse->fc->kq = kqueue();
> if (fuse->fc->kq == -1)
> return (-1);
> @@ -156,6 +159,9 @@ fuse_mount(const char *dir, unused struc
> struct fuse_chan *fc;
> const char *errcause;
>
> + if (dir == NULL)
> + return (NULL);
> +
> fc = calloc(1, sizeof(*fc));
> if (fc == NULL)
> return (NULL);
> @@ -197,9 +203,9 @@ bad:
> }
>
> void
> -fuse_unmount(const char *dir, unused struct fuse_chan *ch)
> +fuse_unmount(const char *dir, struct fuse_chan *ch)
> {
> - if (ch->dead)
> + if (ch == NULL || ch->dead)
> return;
>
> if (unmount(dir, MNT_UPDATE) == -1)
> @@ -207,7 +213,7 @@ fuse_unmount(const char *dir, unused str
> }
>
> int
> -fuse_is_lib_option(unused const char *opt)
> +fuse_is_lib_option(const char *opt)
> {
> return (fuse_opt_match(fuse_core_opts, opt));
> }
> @@ -215,6 +221,9 @@ fuse_is_lib_option(unused const char *op
> int
> fuse_chan_fd(struct fuse_chan *ch)
> {
> + if (ch == NULL)
> + return (-1);
> +
> return (ch->fd);
> }
>
> @@ -227,17 +236,20 @@ fuse_get_session(struct fuse *f)
> int
> fuse_loop_mt(unused struct fuse *fuse)
> {
> - return (0);
> + return (-1);
> }
>
> struct fuse *
> fuse_new(struct fuse_chan *fc, unused struct fuse_args *args,
> const struct fuse_operations *ops, unused size_t size,
> - unused void *userdata)
> + void *userdata)
> {
> struct fuse *fuse;
> struct fuse_vnode *root;
>
> + if (fc == NULL || ops == NULL)
> + return (NULL);
> +
> if ((fuse = calloc(1, sizeof(*fuse))) == NULL)
> return (NULL);
>
> @@ -275,8 +287,11 @@ fuse_daemonize(unused int foreground)
> }
>
> void
> -fuse_destroy(unused struct fuse *f)
> +fuse_destroy(struct fuse *f)
> {
> + if (f == NULL)
> + return;
> +
> close(f->fc->fd);
> free(f->fc->dir);
> free(f->fc);
> @@ -322,7 +337,7 @@ fuse_remove_signal_handlers(unused struc
> }
>
> int
> -fuse_set_signal_handlers(unused struct fuse_session *se)
> +fuse_set_signal_handlers(struct fuse_session *se)
> {
> sigse = se;
> signal(SIGHUP, ifuse_get_signal);
> @@ -344,7 +359,7 @@ dump_help(void)
> static void
> dump_version(void)
> {
> - fprintf(stderr, "FUSE library version %i\n", FUSE_USE_VERSION);
> + fprintf(stderr, "FUSE library version %i\n", FUSE_VERSION);
> }
>
> static int
> @@ -453,6 +468,9 @@ fuse_version(void)
> void
> fuse_teardown(struct fuse *fuse, char *mp)
> {
> + if (fuse == NULL || mp == NULL)
> + return;
> +
> fuse_unmount(mp, fuse->fc);
> fuse_destroy(fuse);
> }
> @@ -500,10 +518,8 @@ int
> fuse_main(int argc, char **argv, const struct fuse_operations *ops, void
> *data)
> {
> struct fuse *fuse;
> - char *mp = NULL;
> - int mt;
>
> - fuse = fuse_setup(argc, argv, ops, sizeof(*ops), &mp, &mt, data);
> + fuse = fuse_setup(argc, argv, ops, sizeof(*ops), NULL, NULL, data);
> if (!fuse)
> return (-1);
>
>
