Re: [RFC][PATCH -mm take4 4/6] using symlink for the net_device

Andrew Morton
2007-04-20T04:13:00Z

On Wed, 18 Apr 2007 21:11:14 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
>
> We use symbolic link for net_device.

As Stephen said, please fully document the new interfaces in netconsole.txt.

Please also cc netdev@vger.kernel.org on all networking-related patches.

> +static char *make_netdev_class_name(char *netdev_name);
> +static int netconsole_event(struct notifier_block *this, unsigned long event,
> + void *ptr);

Please try order things in a way which minimises the number of
forward-declarations, as long as such ordering doesn't make the code
illogical (it usually doesn't).

> static int miscdev_configured;
>
> @@ -274,12 +277,77 @@ static struct miscdevice netconsole_misc
> .name = "netconsole",
> };
>
> +static struct notifier_block netconsole_notifier = {
> + .notifier_call = netconsole_event,
> +};
> +
> static int setup_target_sysfs(struct netconsole_target *nt)
> {
> + int retval = 0;
> + char *name;
> +
> kobject_set_name(&nt->obj, "port%d", nt->id);
> nt->obj.parent = &netconsole_miscdev.this_device->kobj;
> nt->obj.ktype = &target_ktype;
> - return kobject_register(&nt->obj);
> + retval = kobject_register(&nt->obj);
> + name = make_netdev_class_name(nt->np.dev_name);
> + if (IS_ERR(name))
> + return PTR_ERR(name);
> + retval = sysfs_create_link(&nt->obj, &nt->np.dev->dev.kobj, name);
> + kfree(name);
> +
> + return retval;
> +}
> +
> +static char *make_netdev_class_name(char *netdev_name)
> +{
> + int size;
> + char *name;
> + char *netdev_class_prefix = "net:";
> +
> + size = strlen(netdev_class_prefix) + strlen(netdev_name) + 1;
> + name = kmalloc(size, GFP_KERNEL);
> + if (!name) {
> + printk(KERN_ERR "netconsole: kmalloc() failed!\n");
> + return ERR_PTR(-ENOMEM);
> + }
> + strcpy(name, netdev_class_prefix);
> + strcat(name, netdev_name);
> +
> + return name;
> +}

I think this whole function can be replaced by one call to kasprintf()

> +static int netconsole_event(struct notifier_block *this, unsigned long event,
> + void *ptr)
> +{
> + int error = 0;
> + char *old_link_name = NULL, *new_link_name = NULL;
> + struct netconsole_target *nt;
> + struct net_device *dev = ptr;
> +
> + if (event == NETDEV_CHANGENAME) {
> + spin_lock(&target_list_lock);
> + list_for_each_entry(nt, &target_list, list) {
> + if (nt->np.dev != dev)
> + continue;
> + new_link_name = make_netdev_class_name(dev->name);
> + old_link_name =
> + make_netdev_class_name(nt->np.dev_name);

The error return from make_netdev_class_name() is being ignored here.

> + sysfs_remove_link(&nt->obj, old_link_name);
> + error = sysfs_create_link(&nt->obj,
> + &nt->np.dev->dev.kobj,
> + new_link_name);
> + if (error)
> + printk(KERN_ERR "can't create link: %s\n",
> + new_link_name);
> + strcpy(nt->np.dev_name, dev->name);
> + kfree(new_link_name);
> + kfree(old_link_name);
> + }
> + spin_unlock(&target_list_lock);
> + }
> +
> + return NOTIFY_DONE;
> }