From patchwork Fri Jul 7 10:04:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kalle Valo X-Patchwork-Id: 9829883 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3B317602BD for ; Fri, 7 Jul 2017 10:05:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2B26B1FFC9 for ; Fri, 7 Jul 2017 10:05:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1E4B028630; Fri, 7 Jul 2017 10:05:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 590841FFC9 for ; Fri, 7 Jul 2017 10:05:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Content-ID:In-Reply-To: References:Message-ID:Date:Subject:To:From:Reply-To:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Z403ghRsut3CdhidX8QfCP28h3Ty+lvyQ2JB7LXq0LE=; b=JZ0Z6z7wy+xr2S TfZpMjltn0feG+LB6+0BRO83cxQh08l60S+S3snAtpsmiokZUip2GaWMYB71NeOtaTwrmd6KPa3Ie UN4YTxxC+hHyVrnfFOH3F8XZwTV/hhINSt525TVHTzV+KGoatZ9im2ItCMHP+CEX2rFtJK2Q5F4Cq J2ZSzsA5IhHzsP2fC8irvnC0Ti/n+iuzD7ZJ92hOhOUrlEBxRArOLKRwjJXTR7LUr/jOQluybibAs 0TyqpuM0oWCQ7QFMRBp1hV+qHkhoT78uhn68rKifh+R49pEw1451Z7i9t6TlI15eRYIrd476fdb+c 7XTYR0jbhs5Wdt6EsKPw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dTQ85-0001v5-Rj; Fri, 07 Jul 2017 10:04:53 +0000 Received: from sabertooth02.qualcomm.com ([65.197.215.38]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dTQ83-0001t8-1z for ath10k@lists.infradead.org; Fri, 07 Jul 2017 10:04:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=qca.qualcomm.com; i=@qca.qualcomm.com; q=dns/txt; s=qcdkim; t=1499421890; x=1530957890; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=JR7PXeXnnW6tJP9VmnD5sGby+EVcusJuwahEJepzRVM=; b=zIyJV20GgXYnr+aT8oJe38yDhjvp4/BW0nIXIUZTCO2OnP/aeK0F20/e 6UEsCXEq5WpKVIegF/BKPX3DObE0z6iJVmNIYU80wb2W50Dbzib7r9SG+ s807AgIexRr+acRTIMINt43/zxxBWuHaua1VxU/sGntNsxI6NED9s5K7p k=; X-IronPort-AV: E=Sophos;i="5.40,322,1496127600"; d="scan'208";a="110985980" Received: from unknown (HELO Ironmsg04-L.qualcomm.com) ([10.53.140.111]) by sabertooth02.qualcomm.com with ESMTP; 07 Jul 2017 03:04:28 -0700 X-IronPort-AV: E=McAfee;i="5800,7501,8583"; a="1382888915" X-MGA-submission: =?us-ascii?q?MDE5VHQ/5CKHQkwvjGypubP+lN8I8nUvWpxpcp?= =?us-ascii?q?j/jyVofZ0LlkupF8W0FOmLJXQ/JzUi+X/rhymAQlbrBakhpyJLlSDUxO?= =?us-ascii?q?gm/v81c2oJ0ewep5DWNd92ko0za1o6XvfyR0DS9iV32z+oSCj/Gdse+w?= =?us-ascii?q?rX?= Received: from nasanexm02a.na.qualcomm.com ([10.85.0.41]) by Ironmsg04-L.qualcomm.com with ESMTP/TLS/RC4-SHA; 07 Jul 2017 03:04:28 -0700 Received: from euamsexm01e.eu.qualcomm.com (10.251.127.42) by nasanexm02a.na.qualcomm.com (10.85.0.41) with Microsoft SMTP Server (TLS) id 15.0.1178.4; Fri, 7 Jul 2017 03:04:28 -0700 Received: from euamsexm01a.eu.qualcomm.com (10.251.127.40) by euamsexm01e.eu.qualcomm.com (10.251.127.42) with Microsoft SMTP Server (TLS) id 15.0.1178.4; Fri, 7 Jul 2017 12:04:24 +0200 Received: from euamsexm01a.eu.qualcomm.com ([10.251.127.40]) by euamsexm01a.eu.qualcomm.com ([10.251.127.40]) with mapi id 15.00.1178.000; Fri, 7 Jul 2017 12:04:24 +0200 From: Kalle Valo To: Erik Stromdahl Subject: Re: ath10k: ret used but uninitialized Thread-Topic: ath10k: ret used but uninitialized Thread-Index: AQHS9whp4PtXL3F45UqCL+3Dc/PNhA== Date: Fri, 7 Jul 2017 10:04:24 +0000 Message-ID: <87bmowd0mh.fsf@kamboji.qca.qualcomm.com> References: In-Reply-To: (Erik Stromdahl's message of "Thu, 6 Jul 2017 22:01:57 +0200") Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.251.52.12] Content-ID: <3A66E09684FF0242BCD94863EA28B1E8@qualcomm.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170707_030451_245675_94838C70 X-CRM114-Status: GOOD ( 14.99 ) X-BeenThere: ath10k@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Arnd Bergmann , "netdev@vger.kernel.org" , linux-wireless , Linux Kernel Mailing List , "ath10k@lists.infradead.org" , Geert Uytterhoeven Sender: "ath10k" Errors-To: ath10k-bounces+patchwork-ath10k=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Erik Stromdahl writes: >> With gcc 4.1.2: >> >> drivers/net/wireless/ath/ath10k/sdio.c: In function >> ‘ath10k_sdio_mbox_rxmsg_pending_handler’: >> drivers/net/wireless/ath/ath10k/sdio.c:676: warning: ‘ret’ may be used >> uninitialized in this function >> >>> + >>> + *done = true; >>> + >>> + /* Copy the lookahead obtained from the HTC register table into our >>> + * temp array as a start value. >>> + */ >>> + lookaheads[0] = msg_lookahead; >>> + >>> + timeout = jiffies + SDIO_MBOX_PROCESSING_TIMEOUT_HZ; >> >> Although very unlikely due to the long timeout, if the code is preempted here, >> and the loop below never entered, ret will indeed be uninitialized. >> >> It's unclear to me what the proper initialization would be, though, so >> that's why I didn't send a patch. >> > I think it would be best to use 0 as initial value of ret in this case. > This will make all other interrupts be processed in a normal way. > > Kalle: Should I create a new patch (initializing ret with zero)? Yes, please send a new patch fixing this. But I don't like that much with the style of initialising ret to zero, it tends to hide things. Instead my preference is something like below where the error handling is more explicit and easier to find where it's exactly failing. But that's just an example how I would try to solve it, it still lacks the handling of -ECANCEL etc. --- Kalle Valo diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c index 859ed870bd97..19a53e577932 100644 --- a/drivers/net/wireless/ath/ath10k/sdio.c +++ b/drivers/net/wireless/ath/ath10k/sdio.c @@ -689,8 +689,10 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar, */ ret = ath10k_sdio_mbox_rx_alloc(ar, lookaheads, n_lookaheads); - if (ret) - break; + if (ret) { + ath10k_warn(ar, "failed to ....: %d", ret); + return ret; + } if (ar_sdio->n_rx_pkts >= 2) /* A recv bundle was detected, force IRQ status @@ -709,8 +711,10 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar, lookaheads, &n_lookaheads); - if (!n_lookaheads || ret) - break; + if (!n_lookaheads || ret) { + ath10k_warn(ar, "failed to ...."); + return ret; + } /* For SYNCH processing, if we get here, we are running * through the loop again due to updated lookaheads. Set @@ -721,11 +725,7 @@ static int ath10k_sdio_mbox_rxmsg_pending_handler(struct ath10k *ar, *done = false; } - if (ret && (ret != -ECANCELED)) - ath10k_warn(ar, "failed to get pending recv messages: %d\n", - ret); - - return ret; + return 0; } static int ath10k_sdio_mbox_proc_dbg_intr(struct ath10k *ar)