Messages in this thread | | | Date | Mon, 22 Jul 2002 23:04:45 -0600 | From | Richard Gooch <> | Subject | Re: [PATCH] 2.5.27 devfs |
| |
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/
| |