Re: [PATCH] 2.5.27 devfs

From: Richard Gooch (rgooch@ras.ucalgary.ca)
Date: Tue Jul 23 2002 - 00:04:45 EST


Marcin Dalecki writes:
> Richard Gooch wrote:
> > Marcin Dalecki writes:
> >
> >>Kill two inlines which are notwhere used and which don't make sense
> >>in the case someone is not compiling devfs at all.
> >
> >
> > Rejected. Linus, please don't apply this bogus patch. External patches
> > and drivers rely on the inline stubs so that #ifdef CONFIG_DEVFS_FS
> > isn't needed.
>
> Dare to actually *name* one of them?

Apart from my own sdmany patch, other people have contacted me about
this feature and said they were making use of it. I don't bother
tracking everyone who uses my code. I've got better things to do.

In any case, I don't *need* to justify it to you, particularly not
when the compiler completely optimises the inline stubs away. There is
*zero* benefit to removing them, and doing so only breaks external
code.

> You didn't think doing devfs_fs_kernel.h. One simple sample from there:
>
> devfs_get_maj_min(devfs_get_handle_from_inode((inode))

You've managed to pick a function that I'm not thrilled about
either. But it has been necessary.

> Everybody would expect the following to be only a single function:
>
> extern devfs_handle_t devfs_get_handle (devfs_handle_t dir, const char
> extern devfs_handle_t devfs_find_handle (devfs_handle_t dir, const char

devfs_get_handle() is the preferred interface. For compatibility
reasons, devfs_find_handle() remains. However, if you look at the
implementation for devfs_find_handle(), you'll notice that it's marked
for removal. But I believe in stable interfaces, so I want to give
people time to transition.

> And it was of course too hard to unify ops and handle:
>
> extern void *devfs_get_ops (devfs_handle_t de);
> extern void devfs_put_ops (devfs_handle_t de);

Huh? They are different animals.

> You couldn't resist adding the redundant devfs_ prefix overall in the
> kernel:
>
> extern devfs_register_chrdev (unsigned int major, const char *name,
> struct file_operations *fops);
> extern int devfs_register_blkdev (unsigned int major, const char *name,
> struct block_device_operations *bdops);
> extern int devfs_unregister_chrdev (unsigned int major, const char *name);
> extern int devfs_unregister_blkdev (unsigned int major, const char *name);

These do subtly different things than the non "devfs_" versions.

> Three different allocators and deallocators for one single subsystem,
> preserving the illusion that there is in linux a real difference between
> major and minor numbers...
>
> extern int devfs_alloc_major (char type);
> extern void devfs_dealloc_major (char type, int major);
> extern kdev_t devfs_alloc_devnum (char type);
> extern void devfs_dealloc_devnum (char type, kdev_t devnum);

Well, there *is* a difference between major and minor numbers. The two
different interfaces service different driver requirements.
Fortunately, one interface is built on top of the other.

> extern int devfs_alloc_unique_number (struct unique_numspace *space);
> extern void devfs_dealloc_unique_number (struct unique_numspace *space,
> int number);

These are completely unrelated to device numbers, so there's no point
even comparing them.

> If flags are invalid -> add an invalid flag! instead of value return
> through pointer.
>
> static inline int devfs_get_flags (devfs_handle_t de, unsigned int *flags)
> {
> return 0;
> }

That's a spurious objection. The return value indicates whether the
entry was valid or not. That is quite separate from flag values.
Mixing data types by overloading the return value is not a sensible
approach.

                                Regards,

                                        Richard....
Permanent: rgooch@atnf.csiro.au
Current: rgooch@ras.ucalgary.ca
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Jul 23 2002 - 22:00:42 EST