Re: [PATCHv14 2/3] usb: USB Type-C connector class

From: Greg KH
Date: Thu Jan 05 2017 - 11:41:52 EST


On Thu, Jan 05, 2017 at 05:54:02PM +0200, Mika Westerberg wrote:
> > +
> > + ret = device_register(&alt->dev);
> > + if (ret) {
> > + int i;
> > +
> > + dev_err(parent, "failed to register alternate mode (%d)\n",
> > + ret);
> > +
> > + put_device(&alt->dev);
> > +
> > + for (i = 0; i < alt->n_modes; i++)
> > + kfree(alt->modes[i].desc);
> > + kfree(alt);
>
> Just checking: ->release() is not called when device_register() fails
> and that's why you release memory here?

If so, that's wrong. Please see the very good documentation for
device_register in the kernel itself, which says:

* NOTE: _Never_ directly free @dev after calling this function, even
* if it returned an error! Always use put_device() to give up the
* reference initialized in this function instead.

Please read the rest of the documentation there as well, it should
answer all of these types of questions...

Sometimes I wonder why I even wrote that stuff if no one ever reads
it...

Heikki, did you get others at Intel to review this? I don't see their
signed-off-by: on the patches. Please use the internal Intel kernel
developer mailing list for this type of thing, I thought I said I was
going to require that before I would accept these patches after the last
round of review I did.

I'm not going to look at these until you do that, sorry. You have
access to good resources, please use them and don't abuse the community
reviewers for basic things like this.

thanks,

greg k-h