[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <24143.1493893342@warthog.procyon.org.uk>
Date: Thu, 04 May 2017 11:22:22 +0100
From: David Howells <dhowells@...hat.com>
To: Rasmus Villemoes <linux@...musvillemoes.dk>
Cc: dhowells@...hat.com, viro@...iv.linux.org.uk,
linux-fsdevel@...r.kernel.org, linux-nfs@...r.kernel.org,
linux-kernel@...r.kernel.org, mszeredi@...hat.com
Subject: Re: [PATCH 3/9] VFS: Introduce a mount context
Rasmus Villemoes <linux@...musvillemoes.dk> wrote:
> > + if (fs_type->fsopen && fs_type->mc_size < sizeof(*mc))
> > + BUG();
>
> So ->mc_size can be 0 (i.e. not explicitly initialized) if fs_type does
> not have ->fsopen. OK.
I need to be able to handle filesystems that don't support this yet. Once all
filesystems support this, I would be able to take away the max_t() thing.
> > + if (!src->ops->dup)
> > + return ERR_PTR(-ENOTSUPP);
> > +
> > + mc = kmemdup(src, src->fs_type->mc_size, GFP_KERNEL);
>
> So this assumes that vfs_dup_mount_context is only used if ->mc_size is
> explicitly initialized. A max_t here as well probably wouldn't hurt.
If you don't provide an ->fsopen() op, you can't set src->ops, you don't see a
mount context and you can't call this function. If you did supply an
->fsopen() op, the BUG() would've got you if you didn't set ->mc_size.
> > + unsigned short mc_size; /* Size of mount context to allocate */
>
> Any particular reason to use a short? The struct doesn't pack any better.
But it leaves a hole someone else can use. I try not to use fields larger
than I need to.
David
Powered by blists - more mailing lists