linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: Jeff Garzik <jgarzik@redhat.com>
Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org,
	chas@cmf.nrl.navy.mil, rolandd@cisco.com, dwmw2@infradead.org,
	gregkh@suse.de
Subject: Re: [git patches 1/2] warnings: attack valid cases spotted by warnings
Date: Tue, 17 Jul 2007 14:53:02 -0700	[thread overview]
Message-ID: <ada8x9epuip.fsf@cisco.com> (raw)
In-Reply-To: <20070717214239.GF28448@devserv.devel.redhat.com> (Jeff Garzik's message of "Tue, 17 Jul 2007 17:42:39 -0400")

 >     drivers/infiniband/hw/mthca/mthca_qp: kill uninit'd var warning
 >     
 >     drivers/infiniband/hw/mthca/mthca_qp.c: In function
 >       ‘mthca_tavor_post_send’:
 >     drivers/infiniband/hw/mthca/mthca_qp.c:1594: warning: ‘f0’ may be used
 >       uninitialized in this function
 >     drivers/infiniband/hw/mthca/mthca_qp.c: In function
 >       ‘mthca_arbel_post_send’:
 >     drivers/infiniband/hw/mthca/mthca_qp.c:1949: warning: ‘f0’ may be used
 >       uninitialized in this function
 >     
 >     Initializing 'f0' is not strictly necessary in either case, AFAICS.
 >     
 >     I was considering use of uninitialized_var(), but looking at the
 >     complex flow of control in each function, I feel it is wiser and
 >     safer to simply zero the var and be certain of ourselves.
 >     
 >     Signed-off-by: Jeff Garzik <jeff@garzik.org>

I don't really like this.  These functions are in the hottest, most
latency-sensitive code path of this driver, which is used by people
who care about nanoseconds.  I'm quite confident that the code is
correct as written, and it really feels wrong to me to add bloat to
the fastpath just to cover up a shortcoming of gcc.

And I don't like using uninitialized_var() here for a similar
reason... the functions are complex and I would prefer to avoid hiding
future bugs that may creep in.  In fact adding the initialization to 0
has a similar effect, since it shuts up the compiler even if the logic
in the function gets screwed up.

On the other hand I definitely sympathize with the desire to have a
warning-free build so that real warnings are more visible.  I guess I
could live with the uninitialized_var() patch, although I would feel a
little sad.

 - R.

  parent reply	other threads:[~2007-07-17 21:53 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-17 21:42 [git patches 1/2] warnings: attack valid cases spotted by warnings Jeff Garzik
2007-07-17 21:49 ` [git patches 2/2] warnings: use uninitialized_var() Jeff Garzik
2007-07-18 11:30   ` Adrian Bunk
2007-07-17 21:53 ` Roland Dreier [this message]
2007-07-17 22:10   ` [git patches 1/2] warnings: attack valid cases spotted by warnings Jeff Garzik
2007-07-17 22:17     ` Jeff Garzik
2007-07-18  2:35     ` Roland Dreier
2007-07-18  2:46       ` Roland Dreier
2007-07-18  4:00         ` Linus Torvalds
2007-07-18  4:18           ` Roland Dreier
2007-07-18  5:12             ` Linus Torvalds
2007-07-18 17:37               ` Roland Dreier
2007-07-18 18:02                 ` Linus Torvalds
2007-07-18  2:56       ` Linus Torvalds
2007-07-18  3:09         ` Roland Dreier
2007-07-18  3:29           ` Jeff Garzik
2007-07-17 22:19   ` Andrew Morton
2007-07-17 22:25     ` Linus Torvalds
2007-07-18  2:46 ` Greg KH
2007-07-18 20:03   ` Jeff Garzik
2007-07-18 22:07     ` Greg KH

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=ada8x9epuip.fsf@cisco.com \
    --to=rdreier@cisco.com \
    --cc=akpm@linux-foundation.org \
    --cc=chas@cmf.nrl.navy.mil \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@suse.de \
    --cc=jgarzik@redhat.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rolandd@cisco.com \
    --cc=torvalds@linux-foundation.org \
    /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;
as well as URLs for NNTP newsgroup(s).