> >>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >>> + if (!priv)
> >>> + return -ENOMEM;
> >>> +
> >>> + priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
> >>> + if (!priv->hwmon_name)
> >>> + return -ENODEV;
> >>
> >> Do you really need to make a copy of the device name?
> >> Why not simply priv->hwmon_name = dev_name(dev) ?
> >
> > Fine by me, but then maybe I don't quite understand why the other
> > drivers duplicate the name, eg. the sfp.c one.
> >
> It's a question of object lifetime. If the original object can go away
> before your object, then you need to make a copy of the name.
> However in our case I don't think priv can live longer than dev.
>
> >> And if devm_kstrdup fails, then most likely you have an out-of-memory
> >> error, so why not return -ENOMEM as usual?
> >
> > Fixed
> >
> >>> +
> >>> + for (i = 0; priv->hwmon_name[i]; i++)
> >>> + if (hwmon_is_bad_char(priv->hwmon_name[i]))
> >>> + priv->hwmon_name[i] = '_';
This is one reason to make a copy. You don't want to apply that to
main name of the device.
> >>> +
> >>> + priv->hwmon_dev =
> >>> + devm_hwmon_device_register_with_info(dev, priv->hwmon_name,
> >>> + phydev,
> >>> + &tja11xx_hwmon_chip_info,
> >>> + NULL);
> >>> +
The second reason is priv is released before dev, but what about
hwmon, especially if somebody has one of the files open? Is the
unregister synchronous?
Andrew
