lkml.org 
[lkml]   [2002]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC] [PATCH] cdrom per drive sysctl's
    kuebelr@email.uc.edu schrieb:
    > currently, users cannot change cdrom sysctls (debug, autoclose, lock,
    > etc.) on a per drive basis. this patch implements a per-drive
    > sysctl/proc interface based on Jens Axboe's TODO list in cdrom.c.

    Seems fine to me. And it reduces size of the module.
    What about user-space apps ? Wouldn't the patch break those apps that try to read stats the old way ? Then It's probably better for linux 2.5.

    > [...]
    > +static void cdrom_register_sysctl(struct cdrom_device_info *cdi) {
    > + struct cdrom_sysctl_table *cst;
    > +
    > + cdi->sysctl_table = kmalloc(sizeof(struct cdrom_sysctl_table), GFP_KERNEL);
    > + if (!cdi->sysctl_table)
    > + goto err;
    > +

    I personally would use:

    if ( (cdi->sysctl_table = kmalloc(sizeof(struct cdrom_sysctl_table), GFP_KERNEL)) == NULL)
    goto err;

    But that doesn't make any difference for gcc I think.

    > + cst = cdi->sysctl_table;
    > +
    > + /* copy the default settings for the new drive */
    > + memcpy(cst, &cdrom_table, sizeof(struct cdrom_sysctl_table));

    Hm. I don't understand this. Why are you applying the default settings to a new entry hence they're overwritten later ?

    > + cst->options_table[0].data = &cst->options.autoclose;
    > + cst->options_table[1].data = &cst->options.autoeject;
    > + cst->options_table[2].data = &cst->options.debug;
    > + cst->options_table[3].data = &cst->options.lock;
    > + cst->options_table[4].data = &cst->options.check_media;
    > + cst->options_table[5].procname = "info";
    > + cst->options_table[5].extra1 = cdi;
    > + cst->drive_table[0].child = cst->options_table;
    > + cst->drive_table[0].procname = cdi->name;
    > + cst->drive_table[0].ctl_name = kdev_t_to_nr(cdi->dev);
    > + cst->cdrom_table[0].child = cst->drive_table;
    > + cst->dev_table[0].child = cst->cdrom_table;
    > +
    > + cst->sysctl_header = register_sysctl_table(cst->dev_table, 0);
    > + if (cst->sysctl_header)
    > + return;
    > +
    > + kfree(cdi->sysctl_table);
    > +err:
    > + cdinfo(CD_WARNING, "failed to register sysctl table for %s\n", cdi->name);
    > + return;
    > +}
    > [...]

    *Kristian

    :... [snd.science] ...:
    :: _o)
    :: http://www.korseby.net /\\
    :: http://gsmp.sf.net _\_V
    :.........................:
    -
    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: 2005-03-22 13:28    [W:3.295 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site