Re: [RFC] [PATCH] cdrom per drive sysctl's

[Kristian Peters]

[e-mail] 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
  :.........................: