Messages in this thread | | | Date | Tue, 15 Aug 2006 08:18:41 +0400 | From | Alexey Dobriyan <> | Subject | Re: [PATCH] Drop second arg of unregister_chrdev() |
| |
On Mon, Aug 14, 2006 at 08:48:17PM -0700, Andrew Morton wrote: > On Tue, 15 Aug 2006 07:35:22 +0400 > Alexey Dobriyan <adobriyan@gmail.com> wrote: > > > * "name" is trivially unused. > > OK. > > > * Requirement to pass to unregister function anything but cookie you've > > got from register counterpart is wrong. It creates opportunity to > > diverge, it create opportunity for bugs if enforced: > > > > /* > > * XXX(hch): bp->b_count_desired might be incorrect (see > > * xfs_buf_associate_memory for details), > > > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> > > > > * but fortunately > > * the Linux version of kmem_free ignores the len argument.. > > */ > > kmem_free(bp->b_addr, bp->b_count_desired); > > I don't understand that.
p = malloc(size); free(p); is good. You don't have to remember "size" which sometimes nontrivially calculated until free time.
mmio = ioremap(start, size); iounmap(mmio); is good too for same reasons. Agree so far?
Ergo, major = register_chrdev(0, "foo", &foo_fops); unregister_chrdev(major); is good too even if people don't forget to pass the same "foo" in two places. Luckily, unregister_chrdev() ignores name arg, so passing wrong won't do any harm.
> > 64 files changed, 97 insertions(+), 97 deletions(-) > > I do understand that. This'll cause some grief.
In kernel, hardly...
$ grep unregister_chrdev -w -n 2.6.18-rc4-mm1 33126: unregister_chrdev(CPUID_MAJOR, "cpu/cpuid"); 35853: unregister_chrdev(MSR_MAJOR, "cpu/msr"); 35861: unregister_chrdev(MSR_MAJOR, "cpu/msr"); 134610: unregister_chrdev(LP_MAJOR, "lp"); 291207:- unregister_chrdev(hptiop_cdev_major, "hptiop"); 352167:+ unregister_chrdev(major_number, "SerialQT_USB");
Two rejects, 4 fuzzy places.
> I'd suggest that we add a > new unregister_char_dev() or something, and do
But there is register_CHRDEV_region, unregister_CHRDEV_region, alloc_CHRDEV_region, register_CHRDEV. Should I change the spelling of register_chrdev() for a good measure, too?
> static inline unregister_chrdev(unsigned int major, const char *name) > { > return unregister_char_dev(major); > }
> then migrate callers over to unregister_char_dev() in an organised fashion, > via maintainers where poss. > > Then mark unregister_chrdev() deprecated for a while. > > Then nuke it.
- 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/
| |