mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: valdis.kletnieks@vt.edu
To: Nathan Chancellor <natechancellor@gmail.com>
Cc: Aditya Shankar <aditya.shankar@microchip.com>,
	Ganesh Krishna <ganesh.krishna@microchip.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-wireless@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: wilc1000: Remove unnecessary pointer check
Date: Fri, 21 Sep 2018 01:25:32 -0400	[thread overview]
Message-ID: <32473.1537507532@turing-police.cc.vt.edu> (raw)
In-Reply-To: <20180920212648.25181-1-natechancellor@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

On Thu, 20 Sep 2018 14:26:49 -0700, Nathan Chancellor said:
> Clang warns that the address of a pointer will always evaluated as true
> in a boolean context:
>
> drivers/staging/wilc1000/linux_wlan.c:267:20: warning: address of
> 'vif->ndev->dev' will always evaluate to 'true'
> [-Wpointer-bool-conversion]
>         if (!(&vif->ndev->dev))
>             ~  ~~~~~~~~~~~^~~
> 1 warning generated.
>
> Since this statement always evaluates to false due to the logical not,
> remove it.

Often, "just nuke it because it's now dead code" isn't the best answer...

At one time, that was likely intended to be checking whether ->dev was a null
pointer, to make sure we don't pass request_firmware() a null pointer and oops
the kernel, or other things that go pear-shaped....

So the question becomes:   Is it safe to just remove it, or was it intended to
test for something that could  legitimately be null if we've hit an error along
the way (which means we should fix the condition to be proper and acceptable
to both gcc and clang)?



[-- Attachment #2: Type: application/pgp-signature, Size: 486 bytes --]

  reply	other threads:[~2018-09-21  5:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20 21:26 Nathan Chancellor
2018-09-21  5:25 ` valdis.kletnieks [this message]
2018-09-21  5:39   ` Nathan Chancellor
2018-09-21  6:31     ` Ajay Singh
2018-09-21  8:03   ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32473.1537507532@turing-police.cc.vt.edu \
    --to=valdis.kletnieks@vt.edu \
    --cc=aditya.shankar@microchip.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=ganesh.krishna@microchip.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=natechancellor@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®