public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Ian Abbott <abbotti@mev.co.uk>,
	H Hartley Sweeten <hsweeten@visionengravers.com>,
	devel@driverdev.osuosl.org,
	Chase Southwood <chase.southwood@gmail.com>,
	Conrad Meyer <cemeyer@uw.edu>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Fred Akers <knivey@botops.net>
Subject: Re: [PATCH] staging: comedi: drivers: addi-data: hwdrv_apci1500: Change variables that is never used
Date: Fri, 30 Jan 2015 16:26:47 +0300	[thread overview]
Message-ID: <20150130132647.GA6456@mwanda> (raw)
In-Reply-To: <1422481377-31112-1-git-send-email-rickard_strandqvist@spectrumdigital.se>

The more I look at these, the more I don't like these patches.  The
original code looks like:

	foo = inb();

There is a 90% chance that we should just delete the whole line, but the
problem is that deleting the whole line is risky because sometimes
reading from the hardware has side effects.  So there is a 10% chance
that we should change to to:

	inb();

Changing it to that is safer, and that's what we are doing, but there is
a 90% chance that it is the wrong thing to do.

When we silence the warnings in the wrong way, then it makes it harder
for anyone to find the warnings and fix it in the correct way.  I also
would like to fix all the static checker warnings in the kernel, but
there is no point in just "silencing the warnings." we should need to
"fix" them.

It's frustrating that we aren't able to fix this code because we don't
have the hardware, but that's the reality.  Let's, please, leave the
warnings as-is until someone with hardware can fix them properly.

regards,
dan carpenter


  parent reply	other threads:[~2015-01-30 13:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28 21:42 [PATCH] staging: comedi: drivers: addi-data: hwdrv_apci1500: Change variables that is never used Rickard Strandqvist
2015-01-28 21:44 ` Rickard Strandqvist
2015-01-30 13:26 ` Dan Carpenter [this message]
2015-01-30 20:18   ` Dan Carpenter
2015-01-30 20:25     ` Greg Kroah-Hartman
2015-01-30 21:10       ` Dan Carpenter
2015-01-31 13:55         ` Rickard Strandqvist

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=20150130132647.GA6456@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=abbotti@mev.co.uk \
    --cc=cemeyer@uw.edu \
    --cc=chase.southwood@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hsweeten@visionengravers.com \
    --cc=knivey@botops.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rickard_strandqvist@spectrumdigital.se \
    /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