On Mon, 2008-10-27 at 13:40 -0700, Andrew Morton wrote: > On Thu, 23 Oct 2008 10:35:32 +0800 > Ian Kent <raven@xxxxxxxxxx> wrote: > > > This patch further improves autofs mount type usage and provides > > supplementry explanation of the changes made in the previous patch > > "autofs4 - cleanup autofs mount type usage". > > > > Changes introduced in "autofs4 - cleanup autofs mount type usage": > > > > - the type assigned at mount when no type is given is changed > > from 0 to AUTOFS_TYPE_INDIRECT. This was done because 0 and > > AUTOFS_TYPE_INDIRECT were being treated implicitly as the same > > type. > > > > - previously, an offset mount had it's type set to > > AUTOFS_TYPE_DIRECT|AUTOFS_TYPE_OFFSET but the mount control > > re-implementation needs to be able distinguish all three types. > > So this was changed to make the type setting explicit. > > > > - a type AUTOFS_TYPE_ANY was added for use by the re-implementation > > when checking if a given path is a mountpoint. It's not really a > > type as we use this to ask if a given path is a mountpoint in the > > autofs_dev_ioctl_ismountpoint() function. > > > > Changes introduced in this patch: > > > > - macros to set and test the autofs mount types have been added to > > improve readability and make the type usage explicit. > > ^^^^^^^^^^^^^^^^^^^ <<-- ?? > > > - the mount type is used from user space for the mount control > > re-implementtion so, for consistency, all the definitions have > > been moved to the user space include file include/linux/auto_fs4.h. > > > > ... > > > > - if (sbi->type == AUTOFS_TYPE_INDIRECT) > > + if (autofs_type_indirect(sbi->type)) > > spose so. > > > - *type = AUTOFS_TYPE_INDIRECT; > > + set_autofs_type_indirect(*type); > > That's pretty nasty. One doesn't expect a "function" to modify a > variable which was passed by value. > > This interface _requires_ that set_autofs_type_indirect() be > implemented as a macro. > > This didn't improve readability. > > > > > ... > > > > +#define set_autofs_type_indirect(type) (type = AUTOFS_TYPE_INDIRECT) > > You'll find very few places in the kernel pull tricks like this, for > good reasons. The obnoxious exceptions include local_irq_save() and > friends. > > > +#define autofs_type_indirect(type) (type == AUTOFS_TYPE_INDIRECT) > > I guess that's OK. > > But why was it implemented as a macro? It didn't _need_ to be > implemented in cpp - it could have been implemented in C. > > > + > > +#define set_autofs_type_direct(type) (type = AUTOFS_TYPE_DIRECT) > > +#define autofs_type_direct(type) (type == AUTOFS_TYPE_DIRECT) > > + > > +#define set_autofs_type_offset(type) (type = AUTOFS_TYPE_OFFSET) > > +#define autofs_type_offset(type) (type == AUTOFS_TYPE_OFFSET) > > + > > +#define autofs_type_trigger(type) \ > > + (type == AUTOFS_TYPE_DIRECT || type == AUTOFS_TYPE_OFFSET) > > And this one is dangerous. If passed an expression-with-side-effects > it will evaluate that expression either once or twice. Bad. Should be > implemented in C. > OK, more work needed then. Ian -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
