lkml.org 
[lkml]   [2006]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2006-08-15 06:23    [W:0.024 / U:91.356 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site