From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752598AbcBKINR (ORCPT ); Thu, 11 Feb 2016 03:13:17 -0500 Received: from mail-yw0-f194.google.com ([209.85.161.194]:36370 "EHLO mail-yw0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750905AbcBKINM (ORCPT ); Thu, 11 Feb 2016 03:13:12 -0500 MIME-Version: 1.0 In-Reply-To: References: <1455037283-106479-1-git-send-email-heikki.krogerus@linux.intel.com> <1455037283-106479-3-git-send-email-heikki.krogerus@linux.intel.com> <1455113742.8878.14.camel@suse.com> <1455116893.8878.15.camel@suse.de> Date: Thu, 11 Feb 2016 10:13:11 +0200 Message-ID: Subject: Fwd: [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface From: Andy Shevchenko To: Oliver Neukum Cc: Felipe Balbi , Heikki Krogerus , Mathias Nyman , Greg KH , "linux-kernel@vger.kernel.org" , USB Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ---------- Forwarded message ---------- From: Andy Shevchenko 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 On Wed, Feb 10, 2016 at 5:08 PM, Oliver Neukum wrote: > On Wed, 2016-02-10 at 16:24 +0200, Andy Shevchenko wrote: >> On Wed, Feb 10, 2016 at 4:15 PM, Oliver Neukum 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? -- With Best Regards, Andy Shevchenko -- With Best Regards, Andy Shevchenko