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:0.037 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site