Received: by 10.213.65.68 with SMTP id h4csp142744imn; Mon, 12 Mar 2018 09:14:24 -0700 (PDT) X-Google-Smtp-Source: AG47ELt+yJcwRGCffqDohdzap8ZTy1vJidYLC4Px+Ix7RhVeAGDXqJP4J1q3f25gemJkF3N39EcF X-Received: by 10.99.173.79 with SMTP id y15mr7041009pgo.136.1520871264929; Mon, 12 Mar 2018 09:14:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1520871264; cv=none; d=google.com; s=arc-20160816; b=wBW8KgL+DEIyJfTFRMZeLHqKtAttwR0Ex06IhV2eDjkEu3r3NG69sO2Zf0OlL6Oe5I UYs6D0NleLjlQQnYbq82G0AH7yifBIzYelCuVhZaHLZbfLMwS3/uquwYQXCVLBLKDgE6 M+WLb4r/0vXwzbNuvvCJ2xirOSGAukqsC4X4CFetH6ZXYXYj8SmOQX/DJbsxvNwGDrHi BYu49hfl4nDa4rO+p6ylBP6ajIapdN0u8R0mhwIfOQCsMgmuwEKyUyOlDQwpdiiv/gAB Gt90tz3bX26ki9bZN6frqZIJg8/TUJBWHEx+HiA0SSM8xVKEKlYMmAgSysFJ/UmQD9H7 bwKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:to:references:message-id :content-transfer-encoding:cc:date:in-reply-to:from:subject :mime-version:arc-authentication-results; bh=xCAODhalif3vea3dH1QD2KZELXjHef+qOdIi9vVP0Rg=; b=drEXL3w5hZJbGH+UN7YWRShBVXngQhwg+jkzDrdPVarAi/bG9xPnp9SJllaWFqFhAt AJOw47YP18HU26+X9p0tWloBdBSEZiTmHtI4iDrtynreLgAjuwnTMEUIp22cpwaOt7IW VK9NsUrfVYZSe3mPxOzHw5cx2gwmMMOoXEayH+7TFykDMysatUMZ8dGkOPfLV8KfR/zX 41KktakTeYMQTUrx0gA/2y1Kt4uk4g+M8rcGLrCWprj5e2aWgcXpKYDaq/Wx8pzD3+n/ +a4voMQbl25CGZ+Nq2tWkLltoR3VF7PCRcO+Wf/XSfMCn8ZP5NsLICHLiMHLRzxghYIL X/TQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l12-v6si4352495plc.696.2018.03.12.09.14.09; Mon, 12 Mar 2018 09:14:24 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932578AbeCLQMy convert rfc822-to-8bit (ORCPT + 99 others); Mon, 12 Mar 2018 12:12:54 -0400 Received: from coyote.holtmann.net ([212.227.132.17]:47650 "EHLO mail.holtmann.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932205AbeCLQMx (ORCPT ); Mon, 12 Mar 2018 12:12:53 -0400 Received: from marcel-macpro.fritz.box (p4FF9F617.dip0.t-ipconnect.de [79.249.246.23]) by mail.holtmann.org (Postfix) with ESMTPSA id C156BCF351; Mon, 12 Mar 2018 17:19:05 +0100 (CET) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\)) Subject: Re: [PATCH 3/5] Bluetooth: btmrvl: One check less in btmrvl_sdio_card_to_host() after error detection From: Marcel Holtmann In-Reply-To: Date: Mon, 12 Mar 2018 17:12:51 +0100 Cc: linux-bluetooth@vger.kernel.org, Johan Hedberg , LKML , kernel-janitors@vger.kernel.org Content-Transfer-Encoding: 8BIT Message-Id: <609D1B28-2B75-4AB5-B707-FDB5D0500F1A@holtmann.org> References: <8d050f76-aed1-7b6b-8778-a78aef34d4ea@users.sourceforge.net> To: SF Markus Elfring X-Mailer: Apple Mail (2.3445.5.20) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Markus, > One check could be repeated by the btmrvl_sdio_card_to_host() function > during error handling even if the relevant properties can be determined > for the involved variables before by source code analysis. > > * Adjust jump targets so that an extra check can be omitted at the end. > > * Reuse a bit of exception handling better. > > * Delete an initialisation for the local variable "skb" > which became unnecessary with this refactoring. > > Signed-off-by: Markus Elfring > --- > drivers/bluetooth/btmrvl_sdio.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c > index 84c222abf0f7..9854addc8e96 100644 > --- a/drivers/bluetooth/btmrvl_sdio.c > +++ b/drivers/bluetooth/btmrvl_sdio.c > @@ -687,5 +687,5 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > { > u16 buf_len = 0; > int ret, num_blocks, blksz; > - struct sk_buff *skb = NULL; > + struct sk_buff *skb; > u32 type; > @@ -695,16 +695,14 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > > if (!card || !card->func) { > BT_ERR("card or function is NULL!"); > - ret = -EINVAL; > - goto exit; > + goto e_inval; > } > > /* Read the length of data to be transferred */ > ret = btmrvl_sdio_read_rx_len(card, &buf_len); > if (ret < 0) { > BT_ERR("read rx_len failed"); > - ret = -EIO; > - goto exit; > + goto e_io; > } > > blksz = SDIO_BLOCK_SIZE; > @@ -713,8 +711,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > if (buf_len <= SDIO_HEADER_LEN > || (num_blocks * blksz) > ALLOC_BUF_SIZE) { > BT_ERR("invalid packet length: %d", buf_len); > - ret = -EINVAL; > - goto exit; > + goto e_inval; > } > > /* Allocate buffer */ > @@ -722,7 +719,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > if (!skb) { > BT_ERR("No free skb"); > ret = -ENOMEM; > - goto exit; > + goto increment_counter; > } > > if ((unsigned long) skb->data & (BTSDIO_DMA_ALIGN - 1)) { > @@ -738,8 +735,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > num_blocks * blksz); > if (ret < 0) { > BT_ERR("readsb failed: %d", ret); > - ret = -EIO; > - goto exit; > + goto free_skb; > } > > /* This is SDIO specific header length: byte[2][1][0], type: byte[3] > @@ -753,8 +749,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > if (buf_len > blksz * num_blocks) { > BT_ERR("Skip incorrect packet: hdrlen %d buffer %d", > buf_len, blksz * num_blocks); > - ret = -EIO; > - goto exit; > + goto free_skb; > } > > type = payload[3]; > @@ -797,12 +792,18 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv) > break; > } > > -exit: > - if (ret) { > - hdev->stat.err_rx++; > - kfree_skb(skb); > - } > + return 0; > + > +free_skb: > + kfree_skb(skb); > +e_io: > + ret = -EIO; > + goto increment_counter; > > +e_inval: > + ret = -EINVAL; > +increment_counter: > + hdev->stat.err_rx++; > return ret; Nope! This is not easier to read for me. This goto exit jumping and I hate that. Actually to be honest this kind of goto jumping makes my brain hurt and I want to avoid it. Regards Marcel