On Mon, Oct 07, 2019 at 12:36:10PM +0000, Biju Das wrote:
> > err_unreg_port:
> >     typec_unregister_port(hd3ss3220->port);
> > err_put_role:
> >     usb_role_switch_put(hd3ss3220->role_sw);
> > err_put_handle:
> >     fwnode_handle_put(foo bar);
> >
> >     return ret;
> > The rule behind this style of error handling is that you just have to keep 
> > track
> > of the most recently allocated resource and at the bottom you free them in
> > the reverse order from how you allocated them.  Here we had allocated -
> > >role_sw but the typec_register_port() so we do goto free_role_sw;  Now
> > people can guess what the goto does because the name is descriptive and
> > since it matches the most recently allocated resource that means it's okay.
> 
> Yes I agree. But In this case, only one error label is sufficient.

Yes.  You could fix the leak by passing an invalid pointer to
typec_unregister_port() but that way is asking for trouble in the
future...  These are the kinds of bugs I fix all the time because I'm
working with static analysis.  Clearly defined error labels are more
readable and less bug prone.

regards,
dan carpenter
_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-le...@lists.01.org

Reply via email to