> > > > > + pdev = tsm->pdev;
> > > > > + tsm->ops->remove(tsm);
> > > > > + pdev->tsm = NULL;
> > > > > +}
> > > > > +DEFINE_FREE(tsm_remove, struct pci_tsm *, if (_T) tsm_remove(_T))
> > > > > +
> > > > > +static int call_cb_put(struct pci_dev *pdev, void *data,
> > > >
> > > > Is this combination worth while? I don't like the 'and' aspect of it
> > > > and it only saves a few lines...
> > > >
> > > > vs
> > > > if (pdev) {
> > > > rc = cb(pdev, data);
> > > > pci_dev_put(pdev);
> > > > if (rc)
> > > > return;
> > > > }
> > >
> > > I think it is worth it, but an even better option is to just let
> > > scope-based cleanup handle it by moving the variable inside the loop
> > > declaration.
> > I don't follow that lat bit, but will look at next version to see
> > what you mean!
>
> Here is new approach (only compile tested) after understanding that loop
> declared variables do trigger cleanup on each iteration.
Looks good.
>
> [..]
> > I agree it's a slightly odd construction and so might cause confusion.
> > So whilst I think I prefer it to the or_reset() pattern I guess I'll just
> > try and remember why this is odd (should I ever read this again after it's
> > merged!) :)
>
> However, I am interested in these "the trouble with cleanup.h" style
> discussions.
>
> I recently suggested this [1] in another thread which indeed uses
> multiple local variables of the same object to represent the different
> phases of the setup. It was easier there because the code was
> straigtforward to convert to an ERR_PTR() organization.
>
> If there was already an alternative device_add() like this:
>
> struct device *device_add_or_reset(struct device *dev)
>
> That handled the put_device() then you could write:
>
> struct device *devreg __free(device_unregister) = device_add_or_reset(no_free_ptr(dev))
>
> ...and help that common pattern of 'struct device' setup transitions
> from put_device() to device_unregister() at device_add() time.
>
> [1]: http://lore.kernel.org/688bcf40215c3_48e5100d6@xxxxxxxxxxxxxxxxxxxxxxxxx.notmuch
That's definitely interesting (in a fairly good way) as anything to stop people
introducing bugs around the device_add() stuff would be welcome. It'll take a bit
of getting used to though. Maybe make it more explicit device_add_or_put()?
Naming hard as normal..
>
Jonathan
