From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v2] USB: serial: io_edgeport: mark expected switch fall-throughs From: Johan Hovold Message-Id: <20190502144812.GW26546@localhost> Date: Thu, 2 May 2019 16:48:12 +0200 To: Greg Kroah-Hartman Cc: "Gustavo A. R. Silva" , Johan Hovold , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Kees Cook List-ID: T24gVGh1LCBNYXkgMDIsIDIwMTkgYXQgMDQ6NDA6NDFQTSArMDIwMCwgR3JlZyBLcm9haC1IYXJ0 bWFuIHdyb3RlOgo+IE9uIFRodSwgTWF5IDAyLCAyMDE5IGF0IDA5OjI4OjM3QU0gLTA1MDAsIEd1 c3Rhdm8gQS4gUi4gU2lsdmEgd3JvdGU6Cgo+ID4gPj4+PiBAQCAtMTgxMyw2ICsxODEzLDcgQEAg c3RhdGljIHZvaWQgcHJvY2Vzc19yY3ZkX2RhdGEoc3RydWN0IGVkZ2Vwb3J0X3NlcmlhbCAqZWRn ZV9zZXJpYWwsCj4gPiA+Pj4+ICAJCQkJfQo+ID4gPj4+PiAgCQkJCS8qIEVsc2UsIGRyb3AgdGhy b3VnaCAqLwo+ID4gPj4+PiAgCQkJfQo+ID4gPj4+PiArCQkJLyogRmFsbCB0aHJvdWdoICovCj4g PiA+Pj4+ICAJCWNhc2UgRVhQRUNUX0RBVEE6IC8qIEV4cGVjdCBkYXRhICovCj4gPiA+Pj4KPiA+ ID4+PiBMb29rcyBsaWtlIHlvdSBmb3Jnb3QgdG8gdGFrZSB0aGUgb3JpZ2luYWwgcmV2aWV3IGZl ZWRiYWNrIHlvdSBnb3QgaW50bwo+ID4gPj4+IGFjY291bnQ6Cj4gPiA+Pj4KPiA+ID4+PiAJaHR0 cHM6Ly9sa21sLmtlcm5lbC5vcmcvci84N2sxemY0azI0LmZzZkBtaXJhY3VsaXgubW9yay5ubwo+ ID4gPj4+Cj4gPiA+Pgo+ID4gPj4gT2gsIHRoZSB0aGluZyBpcyB0aGF0IHRoZSBmYWxsLXRocm91 Z2ggY29tbWVudHMgaGF2ZSB0byBiZSBwbGFjZWQgYXQKPiA+ID4+IHRoZSB2ZXJ5IGJvdHRvbSBv ZiB0aGUgY2FzZS4gQWxzbywgYmFzZWQgb24gdGhhdCBmZWVkYmFjaywgdGhpcyB0aW1lCj4gPiA+ PiBJIGxlZnQgdGhlICJFbHNlLCBkcm9wIHRocm91Z2giIGNvbW1lbnQgaW4gcGxhY2UsIHNvIHBl b3BsZSBjYW4gYmUKPiA+ID4+IGluZm9ybWVkIHRoYXQgc3VjaCBmYWxsLXRocm91Z2ggaXMgY29u ZGl0aW9uYWwuCj4gPiA+Pgo+ID4gPj4gV2hhdCBkbyB5b3UgdGhpbmsgYWJvdXQgdGhpczoKPiA+ ID4+Cj4gPiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2Ivc2VyaWFsL2lvX2VkZ2Vwb3J0LmMg Yi9kcml2ZXJzL3VzYi9zZXJpYWwvaW9fZWRnZXBvcnQuYwo+ID4gPj4gaW5kZXggNGNhMzFjMGU0 MTc0Li41MmYyN2ZjODI1NjMgMTAwNjQ0Cj4gPiA+PiAtLS0gYS9kcml2ZXJzL3VzYi9zZXJpYWwv aW9fZWRnZXBvcnQuYwo+ID4gPj4gKysrIGIvZHJpdmVycy91c2Ivc2VyaWFsL2lvX2VkZ2Vwb3J0 LmMKPiA+ID4+IEBAIC0xNzUxLDcgKzE3NTEsNyBAQCBzdGF0aWMgdm9pZCBwcm9jZXNzX3JjdmRf ZGF0YShzdHJ1Y3QgZWRnZXBvcnRfc2VyaWFsICplZGdlX3NlcmlhbCwKPiA+ID4+ICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgZWRnZV9zZXJpYWwtPnJ4U3RhdGUgPSBFWFBFQ1RfSERS MjsKPiA+ID4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgYnJlYWs7Cj4gPiA+PiAg ICAgICAgICAgICAgICAgICAgICAgICB9Cj4gPiA+PiAtICAgICAgICAgICAgICAgICAgICAgICAv KiBvdGhlcndpc2UsIGRyb3Agb24gdGhyb3VnaCAqLwo+ID4gPj4gKyAgICAgICAgICAgICAgICAg ICAgICAgLyogRmFsbCB0aHJvdWdoIC0gb3RoZXJ3aXNlLCBkcm9wIG9uIHRocm91Z2ggKi8KPiA+ ID4+ICAgICAgICAgICAgICAgICBjYXNlIEVYUEVDVF9IRFIyOgo+ID4gPj4gICAgICAgICAgICAg ICAgICAgICAgICAgZWRnZV9zZXJpYWwtPnJ4SGVhZGVyMiA9ICpidWZmZXI7Cj4gPiA+PiAgICAg ICAgICAgICAgICAgICAgICAgICArK2J1ZmZlcjsKPiA+ID4+IEBAIC0xODEzLDYgKzE4MTMsMTEg QEAgc3RhdGljIHZvaWQgcHJvY2Vzc19yY3ZkX2RhdGEoc3RydWN0IGVkZ2Vwb3J0X3NlcmlhbCAq ZWRnZV9zZXJpYWwsCj4gPiA+PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIH0KPiA+ ID4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgLyogRWxzZSwgZHJvcCB0aHJvdWdo ICovCj4gPiA+PiAgICAgICAgICAgICAgICAgICAgICAgICB9Cj4gPiA+PiArICAgICAgICAgICAg ICAgICAgICAgICAvKiBCZXdhcmUgdGhhdCwgY3VycmVudGx5LCB0aGVyZSBhcmUgYXQgbGVhc3Qg dGhyZWUKPiA+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgICAqIGJyZWFrIHN0YXRlbWVudHMg aW4gdGhpcyBjYXNlIGJsb2NrLCBzbyB0aGUKPiA+ID4+ICsgICAgICAgICAgICAgICAgICAgICAg ICAqIGZhbGwtdGhyb3VnaCBtYXJrZWQgYmVsb3cgaXMgTk9UIHVuY29uZGl0aW9uYWwuCj4gPiA+ PiArICAgICAgICAgICAgICAgICAgICAgICAgKi8KPiA+ID4+ICsgICAgICAgICAgICAgICAgICAg ICAgIC8qIEZhbGwgdGhyb3VnaCAqLwo+ID4gPj4gICAgICAgICAgICAgICAgIGNhc2UgRVhQRUNU X0RBVEE6IC8qIEV4cGVjdCBkYXRhICovCj4gPiA+PiAgICAgICAgICAgICAgICAgICAgICAgICBp ZiAoYnVmZmVyTGVuZ3RoIDwgZWRnZV9zZXJpYWwtPnJ4Qnl0ZXNSZW1haW5pbmcpIHsKPiA+ID4+ ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcnhMZW4gPSBidWZmZXJMZW5ndGg7Cj4g PiA+IAo+ID4gPiBJdCdzIGJldHRlciB0aGFuIHYyLCBidXQgSSB0aG91Z2h0IHlvdSBzYWlkIHlv dSB3ZXJlIGdvbm5hIGxvb2sgaW50bwo+ID4gPiByZXN0cnVjdHVyaW5nIHRoZSBjb2RlIHRvIG1h aW50YWluIChvciBldmVuIGltcHJvdmUpIHJlYWRhYmlsaXR5Pwo+ID4gPiAKPiA+IAo+ID4gQXQg Zmlyc3QsIEkgdGhvdWdodCBhYm91dCB0aGF0LCBidXQgbm93IEkgZG9uJ3QgdGhpbmsgdGhhdCdz IHJlYWxpc3RpYy4KPiA+IEknZCB0dXJuIHRoZSBpZi1lbHNlIGludG8gYSBzd2l0Y2gsIGFuZCBi YXNlZCBvbiB0aGUgaGlzdG9yeSBvZiBmZWVkYmFjawo+ID4gb24gdGhpcyBwYXRjaCwgd2Ugd2ls bCBlbmQgdXAgaGF2aW5nIHRoZSBzYW1lIGNvbXBsYWlucyBhYm91dCB0aGUgYnJlYWsKPiA+IHN0 YXRlbWVudHMgaW4gdGhhdCBuZXcgc3dpdGNoIGFuZCB0aGUgcG9zc2liaWxpdHkgb2YgYSBmYWxs LXRocm91Z2ggdG8KPiA+IGNhc2UgRVhQRUNUX0RBVEEuIEF0IHRoZSBlbmQgSSB3b3VsZCBzdGls bCBoYXZlIHRvIGFkZCBhIGNvbW1lbnQgZXhwbGFpbmluZwo+ID4gdGhhdCB0aGUgbGFzdCBmYWxs LXRocm91Z2ggbWFyayBpbiB1bmNvbmRpdGlvbmFsLgo+IAo+IEkgbG92ZSBpdCBob3cgbm8gb25l IGlzIGJsYW1pbmcgdGhlIG9yaWdpbmFsIGF1dGhvciBvZiB0aGlzIGNvZGUgKGkuZS4KPiBtZS4u LikKPgo+IExldCBtZSBzZWUgaWYgSSBjYW4gZml4IGl0IHVwIHRvIGJlIG1vcmUgInNhbmUiLCB0 aGlzIGlzIG15IGZhdWx0LgoKVGhhbmtzLCB0aGF0J2QgYmUgZ3JlYXQuIEkgaGF2ZW4ndCBsb29r ZWQgYXQgaXQgbXlzZWxmIGluIGEgbG9uZyB0aW1lLApidXQganVkZ2luZyBmcm9tIHRoZSBvbGQg dGhyZWFkIGl0IGRpZCBub3Qgc2VlbSBpbXBvc3NpYmxlIGF0IGxlYXN0LgoKR2V0dGluZyByaWQg b2Ygc29tZSBvZiB0aGF0IGRlZXAgbmVzdGluZyB3b3VsZCBiZSBnb29kIGVpdGhlciB3YXkuIDop CgpKb2hhbgo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS,T_DKIMWL_WL_HIGH, USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C24CC04AAA for ; Thu, 2 May 2019 14:48:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D05502089E for ; Thu, 2 May 2019 14:48:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556808487; bh=Qe5twwbXdCNOJCV/x3zC4Et43HpncH7Z5E0BfqtXSvE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=HhQOibeQIUVBakUDRiFw7Z7auDaOGo1mDk+YyBbtouq5cC78REd+taGDBlzDX1ZOM pdyWUn8uJakOp/MSxe5VyP8oYFBG3qSdiq7U0qRTV7arUSfhANFErC5oCcLo8fkQFA 3VKLW/sFe/EewztwXVB9FYtA3tNBN6z3p8+/jps0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726394AbfEBOsG (ORCPT ); Thu, 2 May 2019 10:48:06 -0400 Received: from mail-lj1-f193.google.com ([209.85.208.193]:46677 "EHLO mail-lj1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726197AbfEBOsG (ORCPT ); Thu, 2 May 2019 10:48:06 -0400 Received: by mail-lj1-f193.google.com with SMTP id h21so2401807ljk.13; Thu, 02 May 2019 07:48:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=TRsUL+/lAkniHNhKMMSf6UQnO5lUvvQWskmDhwJ5gVo=; b=aUYgGFQYmPyR/FKPMUqoQFurJk9trKoKIgDz08XEsnvimM6Efhrle30oZPn90o7ksF cxnGwkrY1e/fsIA7K59oZLz62H4qzRRv4Ct+3w0inK6JFY+Hm2Ejxe6Me9eMO1/zjrD+ e+QghwCGm3MGoAu+BOLflbrcT358Qat2DSkbnUIfAnzSqvAePSGYizIdS+0GHwWBPkdX r6Qe730Roq7qo+nZiyiBIs0U7trusbD8BRhZrFeJ1QUhLH55d0Z53Z1aWY0vlMTylSN0 GSgBaTdzN/F3mxtp+SPnTCWLfj7Ipr+gBhffSJNo9agvUT0TR2mzXHeKv+9JFazFj6Bz QRBA== X-Gm-Message-State: APjAAAV2V+J48OQJbUgx77QZ4705T8FDwB8RkN9W1J53Ru0TX7i4vTuZ eYnC7sieooHEFS1YYt/Ei1A= X-Google-Smtp-Source: APXvYqxZsE2YBL/XpPCmdhnPx9C9hLu50MtUOZLaQChBJzwTG5MF++8SRvEv0a/loPukkLR+USB8vQ== X-Received: by 2002:a2e:2b8c:: with SMTP id r12mr2283031ljr.115.1556808483573; Thu, 02 May 2019 07:48:03 -0700 (PDT) Received: from xi.terra (c-74bee655.07-184-6d6c6d4.bbcust.telenor.se. [85.230.190.116]) by smtp.gmail.com with ESMTPSA id m28sm9465085lfc.71.2019.05.02.07.48.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 02 May 2019 07:48:02 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.91) (envelope-from ) id 1hMD0O-0003US-E7; Thu, 02 May 2019 16:48:13 +0200 Date: Thu, 2 May 2019 16:48:12 +0200 From: Johan Hovold To: Greg Kroah-Hartman Cc: "Gustavo A. R. Silva" , Johan Hovold , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Kees Cook Subject: Re: [PATCH v2] USB: serial: io_edgeport: mark expected switch fall-throughs Message-ID: <20190502144812.GW26546@localhost> References: <20190501213329.GA26972@embeddedor> <20190502102608.GS26546@localhost> <20190502135636.GT26546@localhost> <20190502144041.GA17960@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <20190502144041.GA17960@kroah.com> User-Agent: Mutt/1.11.4 (2019-03-13) Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Message-ID: <20190502144812.AbfOPbO1kuWbT2Sqs4cOZVldbS123ZQh1fNlgbx0rdo@z> On Thu, May 02, 2019 at 04:40:41PM +0200, Greg Kroah-Hartman wrote: > On Thu, May 02, 2019 at 09:28:37AM -0500, Gustavo A. R. Silva wrote: > > >>>> @@ -1813,6 +1813,7 @@ static void process_rcvd_data(struct edgeport_serial *edge_serial, > > >>>> } > > >>>> /* Else, drop through */ > > >>>> } > > >>>> + /* Fall through */ > > >>>> case EXPECT_DATA: /* Expect data */ > > >>> > > >>> Looks like you forgot to take the original review feedback you got into > > >>> account: > > >>> > > >>> https://lkml.kernel.org/r/87k1zf4k24.fsf@miraculix.mork.no > > >>> > > >> > > >> Oh, the thing is that the fall-through comments have to be placed at > > >> the very bottom of the case. Also, based on that feedback, this time > > >> I left the "Else, drop through" comment in place, so people can be > > >> informed that such fall-through is conditional. > > >> > > >> What do you think about this: > > >> > > >> diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c > > >> index 4ca31c0e4174..52f27fc82563 100644 > > >> --- a/drivers/usb/serial/io_edgeport.c > > >> +++ b/drivers/usb/serial/io_edgeport.c > > >> @@ -1751,7 +1751,7 @@ static void process_rcvd_data(struct edgeport_serial *edge_serial, > > >> edge_serial->rxState = EXPECT_HDR2; > > >> break; > > >> } > > >> - /* otherwise, drop on through */ > > >> + /* Fall through - otherwise, drop on through */ > > >> case EXPECT_HDR2: > > >> edge_serial->rxHeader2 = *buffer; > > >> ++buffer; > > >> @@ -1813,6 +1813,11 @@ static void process_rcvd_data(struct edgeport_serial *edge_serial, > > >> } > > >> /* Else, drop through */ > > >> } > > >> + /* Beware that, currently, there are at least three > > >> + * break statements in this case block, so the > > >> + * fall-through marked below is NOT unconditional. > > >> + */ > > >> + /* Fall through */ > > >> case EXPECT_DATA: /* Expect data */ > > >> if (bufferLength < edge_serial->rxBytesRemaining) { > > >> rxLen = bufferLength; > > > > > > It's better than v2, but I thought you said you were gonna look into > > > restructuring the code to maintain (or even improve) readability? > > > > > > > At first, I thought about that, but now I don't think that's realistic. > > I'd turn the if-else into a switch, and based on the history of feedback > > on this patch, we will end up having the same complains about the break > > statements in that new switch and the possibility of a fall-through to > > case EXPECT_DATA. At the end I would still have to add a comment explaining > > that the last fall-through mark in unconditional. > > I love it how no one is blaming the original author of this code (i.e. > me...) > > Let me see if I can fix it up to be more "sane", this is my fault. Thanks, that'd be great. I haven't looked at it myself in a long time, but judging from the old thread it did not seem impossible at least. Getting rid of some of that deep nesting would be good either way. :) Johan