From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]) by casper.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1gRGbD-00065F-Qr for ath10k@lists.infradead.org; Mon, 26 Nov 2018 13:06:53 +0000 From: Kalle Valo Subject: Re: [PATCH] ath10k: fix a NULL vs IS_ERR() check References: <20181126081143.tvu55aimdc3safcq@kili.mountain> <8736rob60p.fsf@kamboji.qca.qualcomm.com> <20181126083021.GC3088@unbuntlaptop> Date: Mon, 26 Nov 2018 15:06:33 +0200 In-Reply-To: <20181126083021.GC3088@unbuntlaptop> (Dan Carpenter's message of "Mon, 26 Nov 2018 11:30:22 +0300") Message-ID: <87va4k9eee.fsf@kamboji.qca.qualcomm.com> MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Dan Carpenter Cc: kernel-janitors@vger.kernel.org, Govind Singh , linux-wireless@vger.kernel.org, ath10k@lists.infradead.org Dan Carpenter writes: > On Mon, Nov 26, 2018 at 10:24:38AM +0200, Kalle Valo wrote: >> Dan Carpenter writes: >> >> > The devm_memremap() function doesn't return NULLs, it returns error >> > pointers. >> > >> > Fixes: ba94c753ccb4 ("ath10k: add QMI message handshake for wcn3990 client") >> > Signed-off-by: Dan Carpenter >> > --- >> > drivers/net/wireless/ath/ath10k/qmi.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c >> > index f0429bee35c2..37b3bd629f48 100644 >> > --- a/drivers/net/wireless/ath/ath10k/qmi.c >> > +++ b/drivers/net/wireless/ath/ath10k/qmi.c >> > @@ -931,9 +931,9 @@ static int ath10k_qmi_setup_msa_resources(struct ath10k_qmi *qmi, u32 msa_size) >> > qmi->msa_mem_size = resource_size(&r); >> > qmi->msa_va = devm_memremap(dev, qmi->msa_pa, qmi->msa_mem_size, >> > MEMREMAP_WT); >> > - if (!qmi->msa_va) { >> > + if (IS_ERR(qmi->msa_va)) { >> > dev_err(dev, "failed to map memory region: %pa\n", &r.start); >> > - return -EBUSY; >> > + return PTR_ERR(qmi->msa_va); >> >> That's a very good find! >> >> But how has this even worked before? > > This only changes the error path. Presumably it always succeeds in real > life. Ah, I missed the "!" operator. Yeah, I think you are correct. I'll queue this for -next then. -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k