![]() | |||||||||||||
Messages in this thread |
Hi,
On 6/13/07, Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> From: Keiichi KII <k-keiichi@bx.jp.nec.com>>> This patch contains the following changes.>> create a sysfs entry for netconsole in /sys/class/misc.
> This entry has elements related to netconsole as follows.
> You can change configuration of netconsole(writable attributes such as IP
> address, port number and so on) and check current configuration of netconsole.
>> -+- /sys/class/misc/> |-+- netconsole/> |-+- port1/
> | |--- id [r--r--r--] unique port id
> | |--- local_ip [rw-r--r--] source IP to use, writable
> | |--- local_mac [r--r--r--] source MAC address
> | |--- local_port [rw-r--r--] source port number for UDP packets, writable
> | |--- remote_ip [rw-r--r--] port number for logging agent, writable
> | |--- remote_mac [rw-r--r--] MAC address for logging agent, writable
> | ---- remote_port [rw-r--r--] IP address for logging agent, writable
> |--- port2/
> |--- port3/
[...]
> +static int miscdev_configured;
Is this really required? We just return with error if misc_register()
fails during module init time itself, so it's not really useful ever, is it?
> +static ssize_t show_target_attr(struct kobject *kobj,
[...]
> + if (na->show)> + return na->show(nt, buffer);> + else> + return -EIO;
and
> +static ssize_t store_target_attr(struct kobject *kobj,
[...]
> + if (na->store)> + return na->store(nt, buffer, count);
> + else> + return -EIO;
EIO isn't quite appropriate for the above two cases. EPERM?
> +static int setup_target_sysfs(struct netconsole_target *nt)
> +{> + kobject_set_name(&nt->obj, "port%d", nt->id);
Ah, I see, so this is where we use the ->id member.
> @@ -192,7 +386,9 @@ static void cleanup_netconsole(void)
>> unregister_console(&netconsole);
> list_for_each_entry_safe(nt, tmp, &target_list, list)
> - remove_target(nt);
> + kobject_unregister(&nt->obj);
> + if (miscdev_configured)
> + misc_deregister(&netconsole_miscdev);
Yeah, I suspect we can do away with miscdev_configured here.
We reach this code only if it is known to be true.
> + if (misc_register(&netconsole_miscdev)) {
> + printk(KERN_ERR> + "netconsole: failed to register misc device for "
> + "name = %s, id = %d\n",
> + netconsole_miscdev.name, netconsole_miscdev.minor);
> + return -EIO;
> + } else
> + miscdev_configured = 1;
> +
Please prefer:
err = misc_register(...);
if (err)
...
return err;
That way we avoid the weird EIO and maintain the error code
returned by misc_register().
Satyam
-
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: 2007-06-13 17:53 [from the cache] ©2003-2008 | |||||||||||||