Re: [PATCH] staging: rtl8192u: fix rmmod warn when wlan0 is renamed

[Tong Zhang]

On Mon, Jul 18, 2022 at 5:02 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> On Sun, Jul 17, 2022 at 12:01:57AM -0700, Tong Zhang wrote:
> > +static int rtl8192_usb_netdev_event(struct notifier_block *nb, unsigned long event,
> > +             void *data)
> > +{
> > +     struct net_device *netdev = netdev_notifier_info_to_dev(data);
> > +
> > +     if (netdev->netdev_ops != &rtl8192_netdev_ops)
> > +             goto out;
> > +
> > +     switch (event) {
> > +             case NETDEV_CHANGENAME:
> > +                     rtl8192_debugfs_rename(netdev);
> > +                     break;
> > +
> > +             default:
> > +                     break;
> > +     }
>
> This isn't indented properly.  Don't forget to run ./scripts/checkpatch.pl
> on your patch.
>
> > +
> > +out:
> > +     return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rtl8192_usb_netdev_notifier = {
> > +     .notifier_call = rtl8192_usb_netdev_event,
> > +};
> > +
> > +
>
> Here too.
>
> >  static int __init rtl8192_usb_module_init(void)
> >  {
> >       int ret;
> > @@ -4788,10 +4638,14 @@ static int __init rtl8192_usb_module_init(void)
>
> [ snip ]
>
> > +void rtl8192_debugfs_init(struct net_device *dev)
> > +{
> > +     struct dentry *dir;
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +
> > +     dir = debugfs_create_dir(dev->name, NULL);
> > +     if (IS_ERR_OR_NULL(dir))
> > +             return;
>
> In olden times the debugfs_create_dir() function used to return a mix
> of error pointers and NULL.  But the idea with that function is that
> most people are not supposed to check for errors.  But instead of that
> they added all kind of *buggy* checks.  So then we made it just return
> error pointers.
>
> This code *does* care about it because it uses the
> priv->debugfs_dir->d_name.name in rtl8192_debugfs_rename().
>
> But caring about the debugfs dir and creating a rtl8192_debugfs_rename()
> function is really unusual.  And normally when we have to do something
> unusual that means we are doing something wrong...  :/
>
> Anyway, just check for if (IS_ERR(dir)) {
>
> > +
> > +     debugfs_create_file("stats-rx", 0444, dir, dev, &rtl8192_usb_stats_rx_fops);
> > +     debugfs_create_file("stats-tx", 0444, dir, dev, &rtl8192_usb_stats_tx_fops);
> > +     debugfs_create_file("stats-ap", 0444, dir, dev, &rtl8192_usb_stats_ap_fops);
> > +     debugfs_create_file("registers", 0444, dir, dev, &rtl8192_usb_registers_fops);
> > +
> > +     priv->debugfs_dir = dir;
> > +}
> > +
> > +void rtl8192_debugfs_exit(struct net_device *dev)
> > +{
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +     if (!priv->debugfs_dir)
> > +             return;
> > +     debugfs_remove_recursive(priv->debugfs_dir);
> > +     priv->debugfs_dir = NULL;
> > +}
> > +
> > +void rtl8192_debugfs_rename(struct net_device *dev)
> > +{
> > +     struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);
> > +
> > +     if (!priv->debugfs_dir)
> > +             return;
> > +
> > +     if (!strcmp(priv->debugfs_dir->d_name.name, dev->name))
> > +             return;
> > +
> > +     debugfs_rename(priv->debugfs_dir->d_parent, priv->debugfs_dir,
> > +                     priv->debugfs_dir->d_parent, dev->name);
> > +}
> > +
>
> regards,
> dan carpenter

I modified the patch and sent it as v2.
Thanks and have a good one!
- Tong