linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bernd Petrovitsch <bernd@petrovitsch.priv.at>
To: David Howells <dhowells@redhat.com>
Cc: "Justin P. Mattock" <justinmattock@gmail.com>,
	"Gustavo F. Padovan" <gustavo@padovan.org>,
	linux-kernel@vger.kernel.org, sds@tycho.nsa.gov, lenb@kernel.org,
	linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH 5/5]bluetooth:hci_bcsp Fix operation on 'bcsp->msgq_txseq' may be undefined
Date: Mon, 28 Jun 2010 15:02:38 +0200	[thread overview]
Message-ID: <1277730158.3831.17.camel@thorin> (raw)
In-Reply-To: <7323.1277729576@redhat.com>

On Mon, 2010-06-28 at 13:52 +0100, David Howells wrote:
> Justin P. Mattock <justinmattock@gmail.com> wrote:
> 
> > -		BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq);
> > -		bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> > +		BT_DBG("Sending packet with seqno %u", bcsp->msgq_txseq | ret);
> > +		ret = ++(bcsp->msgq_txseq) & 0x07;
> 
> I don't know what you're trying to do here, but you seem to be trying to send
> the computed value back in time.
> 
> The problem is that the compiler is confused about why a '++' operator makes

It's even worse as that expression is explicitly undefined (and should
be fixed anyways and unconditionally).

> any sense here.  It doesn't.  It should be a '+ 1' instead.  I think what you
> want is:
> 
> 	-	bcsp->msgq_txseq = ++(bcsp->msgq_txseq) & 0x07;
> 	+	bcsp->msgq_txseq = (bcsp->msgq_txseq + 1) & 0x07;

Yes, that's looks like the most probable intention of it - at least for
one who doesn't know the bluetooth code.

	Bernd
-- 
Bernd Petrovitsch                  Email : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at


  reply	other threads:[~2010-06-28 13:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1277621246-10960-1-git-send-email-justinmattock@gmail.com>
2010-06-27  6:47 ` [PATCH 5/5]bluetooth:hci_bcsp Fix operation on 'bcsp->msgq_txseq' may be undefined Justin P. Mattock
2010-06-27  7:31   ` Gustavo F. Padovan
2010-06-28 12:57   ` [PATCH] Bluetooth: Fix abuse of the preincrement operator David Howells
2010-06-28 13:12     ` Gustavo F. Padovan
2010-06-30 20:10       ` David Miller
2010-06-28 17:44     ` Justin P. Mattock
2010-06-28 12:52 ` [PATCH 5/5]bluetooth:hci_bcsp Fix operation on 'bcsp->msgq_txseq' may be undefined David Howells
2010-06-28 13:02   ` Bernd Petrovitsch [this message]
2010-06-28 17:56   ` Justin P. Mattock

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=1277730158.3831.17.camel@thorin \
    --to=bernd@petrovitsch.priv.at \
    --cc=dhowells@redhat.com \
    --cc=gustavo@padovan.org \
    --cc=justinmattock@gmail.com \
    --cc=lenb@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sds@tycho.nsa.gov \
    /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).