On Thu, Feb 11, 2016 at 10:13:11AM +0200, Andy Shevchenko wrote: > ---------- Forwarded message ---------- > From: Andy Shevchenko <[email protected]> > Date: Thu, Feb 11, 2016 at 10:10 AM > Subject: Re: [PATCH 2/3] usb: type-c: USB Type-C Connector System > Software Interface > To: Oliver Neukum <[email protected]> > > > On Wed, Feb 10, 2016 at 5:08 PM, Oliver Neukum <[email protected]> wrote: > > On Wed, 2016-02-10 at 16:24 +0200, Andy Shevchenko wrote: > >> On Wed, Feb 10, 2016 at 4:15 PM, Oliver Neukum <[email protected]> wrote: > >> > On Wed, 2016-02-10 at 13:56 +0200, Andy Shevchenko wrote: > >> >> > +err: > >> >> > + if (i > 0) > >> >> > + for (; i >= 0; i--, con--) > >> >> > + typec_unregister_port(con->port); > >> >> > >> >> Perhaps > >> >> > >> >> while (--i >= 0) { > >> >> ... > >> >> } > >> > > >> > While we are at it. No we should not change the semantics > >> > of conditionals for the sake of appearance. > >> > >> I'm sorry I didn't get you. > >> How this more or less standard pattern to clean up stuff on error path > >> does with conditional semantics? > > > > You change a postdecrement to a predecrement. The highest > > number the loop is executed for is changed. > > I still didn't get. > Variable i is just counter here, > > And it seems there is a bug, since when i == 1, we will have > > i = 1, con == connector[0]: > typec_unregister_port(con->port); > > i = 0, con == connector[1]: > typec_unregister_port(con->port); <<< It wasn't registered yet! > > The correct code should be something like > if (i > 0) > for (--i; i >= 0; i--) {} > > Which > a) makes conditional redundant; > b) classical pattern of while (--i >= 0) {} > > So where am I wrong?
I think Andy has a point here. Thanks, -- heikki
