On Wed, 18 Apr 2007 21:11:14 +0900 Keiichi KII <[EMAIL PROTECTED]> wrote:

> From: Keiichi KII <[EMAIL PROTECTED]>
> 
> 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;
>  }

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to