2012-06-07 20:41:15

by Brian Smith

[permalink] [raw]
Subject: Small Patch for scotest.c

Hi there.
Here is a tiny 3-line patch to scotest.c to get it working. (I've been
trying to get bluez working on a Raspberry Pi, hitting USB problems,
having a workng scotest is helpful). The problem is that bdaddr never
gets initialized and tends to have random data in it, preventing the
client/server end from matching up.

344a345,346
> hci_devba(0, &bdaddr);
>


--
Brian Smith.
"The bells of clocktowers stitch the sleeper's dreams together." - Memory Palace



2012-06-10 17:28:00

by Brian Smith

[permalink] [raw]
Subject: Re: Small Patch for scotest.c

Johan Hedberg wrote:
> Hi Brian,
>
> On Thu, Jun 07, 2012, Brian Smith wrote:
>
>> Hi there.
>> Here is a tiny 3-line patch to scotest.c to get it working. (I've
>> been trying to get bluez working on a Raspberry Pi, hitting USB
>> problems, having a workng scotest is helpful). The problem is that
>> bdaddr never gets initialized and tends to have random data in it,
>> preventing the client/server end from matching up.
>>
>> 344a345,346
>>
>>> hci_devba(0, &bdaddr);
>>>
>>>
>
> Please only send patches in unified diff format. If you want them
> applied upstream they should be created using "git format-patch" and
> preferably sent with "git send-email".
>
> However, in this case I don't think your patch is quite right even if it
> was in the right format. Looking at bdaddr in scotest.c it's a static
> variable and to my understanding those should (according to the C
> standard) always get implicitly initialized to 0 if the code itself
> doesn't do so. At least gcc should follow this, so which compiler are
> you really using?
>
> Furthermore, assuming that this (zero-initialized bdaddr) is how scotest
> behaves for most people (as it's existed many years and you're the first
> one to face the issue), it's the same as using BDADDR_ANY. Therefore,
> I'd just go ahead and remove the bdaddr variable and replace the places
> where it was used with BDADDR_ANY (and please format the patch like I
> described above). Thanks.
>
> Johan
>
>
Hi Johan.
Thanks for taking the time to look at my email and the feedback. Yes,
you are right about the static variable initialization, bdaddr starts as
0:0:0:0:0:0, but my understanding of what hci_devba() does was wrong.
It changes baddrr from 0 to the MAC address of hci0, the local bluetooth
adapter, which does actually seem to make scotest work properly (In
practice it certainly does from the testing I've done. I can imagine
that when both the server and client MAC address are 0/BDADDR_ANY it
could cause problems). So I think the general idea of the patch is right
at least. (It just came from comparing scotest.c with l2test.c really).
Also I'm not the first person to see this, I found this unresolved issue
when googling the problem initially:

http://comments.gmane.org/gmane.linux.bluez.kernel/3122

--
Brian Smith.
"The bells of clocktowers stitch the sleeper's dreams together." - Memory Palace


2012-06-10 08:46:58

by Johan Hedberg

[permalink] [raw]
Subject: Re: Small Patch for scotest.c

Hi Brian,

On Thu, Jun 07, 2012, Brian Smith wrote:
> Hi there.
> Here is a tiny 3-line patch to scotest.c to get it working. (I've
> been trying to get bluez working on a Raspberry Pi, hitting USB
> problems, having a workng scotest is helpful). The problem is that
> bdaddr never gets initialized and tends to have random data in it,
> preventing the client/server end from matching up.
>
> 344a345,346
> > hci_devba(0, &bdaddr);
> >

Please only send patches in unified diff format. If you want them
applied upstream they should be created using "git format-patch" and
preferably sent with "git send-email".

However, in this case I don't think your patch is quite right even if it
was in the right format. Looking at bdaddr in scotest.c it's a static
variable and to my understanding those should (according to the C
standard) always get implicitly initialized to 0 if the code itself
doesn't do so. At least gcc should follow this, so which compiler are
you really using?

Furthermore, assuming that this (zero-initialized bdaddr) is how scotest
behaves for most people (as it's existed many years and you're the first
one to face the issue), it's the same as using BDADDR_ANY. Therefore,
I'd just go ahead and remove the bdaddr variable and replace the places
where it was used with BDADDR_ANY (and please format the patch like I
described above). Thanks.

Johan