2005-10-11 14:26:28

by Claudio Takahasi

[permalink] [raw]
Subject: [Bluez-devel] hcid patch (remote name and connections)

Hi folks/Marcel,

I added the support to remote name and display connections.
This patch is based on dbus.c revision 1.20

There is a command line client attached to test the added services.

Regards,
Claudio

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]


Attachments:
(No filename) (356.00 B)
(No filename) (482.00 B)
bluez-hcid-dbus-client.tar.gz (379.44 kB)
rem_name_connections_01.patch (7.42 kB)
Download all attachments

2005-10-13 09:38:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Hi Claudio,

> This is the patch to add remote name and display connections. I fixed
> a bitwise operation in the error message function and the dbus message
> handler.
>
> Apply the segmentation fault patch before!

I applied both patches, but you should start looking at my changes to
your previous patch. There are some ways to code the if clauses a little
bit better using goto. This will make the code better readable. I might
fix this up later anyhow, but at the moment my Internet connection is
limited.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-12 21:14:04

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Hi,

This is the patch to add remote name and display connections. I fixed a
bitwise operation in the error message function and the dbus message
handler.

Apply the segmentation fault patch before!

Regards,
Claudio.

On 10/12/05, Johan Hedberg <[email protected]> wrote:
>
> Hi,
>
> I missed one place in the code where hci_dbus_data should be allocated
> (thanks to Claudio for noticing). Here's a patch to fix it.
>
> Johan
>
>
>


--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]


Attachments:
(No filename) (588.00 B)
(No filename) (996.00 B)
rem_name_connections_02.patch (7.16 kB)
Download all attachments

2005-10-12 19:38:11

by Johan Hedberg

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Hi,

I missed one place in the code where hci_dbus_data should be allocated
(thanks to Claudio for noticing). Here's a patch to fix it.

Johan


Attachments:
(No filename) (143.00 B)
segfault.patch (820.00 B)
Download all attachments

2005-10-11 22:48:38

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Hi Johan,

> > I'd recommend encapsulating the id in a structure rather than allocating
> > sizeof(guint). It doesn't cost you much time now and if, in the future,
> > you ever need to store any more information then the structure's already
> > there.
> > I noticed you already have a structure struct hci_dbus_data which
> > contains just an integer. Did you mean to use that here or are they
> > logically separate types (that just happen to be the same at the
> > moment)?
>
> The watch code is logically completely separate from the code that uses
> the hci_dbus_data struct (the "id" variable refers to a GIOChannel id
> while in the hci_dbus_data struct id refers to the device id). It is
> also much simpler than the code that uses hci_dbus_data and I don't
> think that anything besides the GIOChannel id needs to be passed to the
> remove_watch function.

the patch is in and I agree with you that it makes no sense to take care
of this issue, because we will no longer use GIOChannel in bluetoothd in
the future.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 22:37:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] Re: hcid patch (remote name and connections)

Hi Steven,

> > It's from hcitool code :)
>
> :-)
>
> Still, that's no reason to propagate the error. Fix it and port the fix
> back to hcitool (and wherever else it's escaped from).
>
> > I will replace by "7", the maximum number of connections. Is it ok?
>
> It wasn't the value I was concerned about. It was the free-floating 10
> in the code twice.

the current API sucks and so we have some more of these crazy numbers
flying around. I am aware of them and the new sysfs based interface will
hopefully solve all our problems.

All of these magic number are from the really beginnings of BlueZ where
it was almost impossible to get any CSR based dongles or PCMCIA cards ;)

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 21:14:01

by Steven Singer

[permalink] [raw]
Subject: Re: [Bluez-devel] Re: hcid patch (remote name and connections)

Claudio Takahasi wrote:
> It's from hcitool code :)

:-)

Still, that's no reason to propagate the error. Fix it and port the fix
back to hcitool (and wherever else it's escaped from).

> I will replace by "7", the maximum number of connections. Is it ok?

It wasn't the value I was concerned about. It was the free-floating 10
in the code twice.

A #define should allay my immediate concerns, however, a better solution
would be to avoid the use of hard-coded constant limits. Is there any way
to retrieve the current number of connections?

Strictly, 7 isn't the maximum number of connections - it's the maximum
number of active slaves a master can have. In theory the number of
simultaneous connections a device can have is limited only by the number
of ACL handles (about 4000). In practice, it's limited by the amount of
memory on the device and the need to service all the connections.

If you have more than one HCI dongle then you can have even more
connections.

In practice, 10 is probably sufficient for today, but by making it a
#define with a self-documenting name, it's easy for someone to change it
in the future.

If the kernel has some limit on the maximum number of connections then
use that (and get it from the kernel or a #include, don't copy the number
into your code).

If you can't get the number of connections from the kernel then that
suggests that there's a kernel API missing.

You may be able to work round the missing API depending on the behaviour
of HCIGETCONNLIST when the array size (N) is too small to hold all the
connections (M).

* If it causes a segmentation fault and terminates your program then
you may be in trouble.

* If it copies the first N connections and returns with conn_num
set to N then test for conn_num being set to N on return and
if it is, double N and try again.

* If it copies the first N connections and returns with conn_num
set to M then make two calls: the first with N = 0 and the second
with N = M.

- Steven
--


This message has been scanned for viruses by BlackSpider MailControl - http://www.blackspider.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 20:07:23

by Claudio Takahasi

[permalink] [raw]
Subject: Re: [Bluez-devel] Re: hcid patch (remote name and connections)

It's from hcitool code :)
I will replace by "7", the maximum number of connections. Is it ok?

Regards,
Claudio

On 10/11/05, Steven Singer <[email protected]> wrote:
>
> Claudio Takahasi wrote:
> > + if (!(cl = malloc(10 * sizeof(*ci) + sizeof(*cl)))) {
> ...
> > + cl->conn_num = 10;
>
> *cough* magic number *cough*
>
> - Steven
> --
>
>
> This message has been scanned for viruses by BlackSpider MailControl -
> http://www.blackspider.com <http://www.blackspider.com>
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by:
> Power Architecture Resource Center: Free content, downloads, discussions,
> and more. http://solutions.newsforge.com/ibmarch.tmpl
> _______________________________________________
> Bluez-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>



--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]


Attachments:
(No filename) (1.00 kB)
(No filename) (1.70 kB)
Download all attachments

2005-10-11 19:55:31

by Johan Hedberg

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Hi Steven,

On Tue, Oct 11, 2005, Steven Singer wrote:
> I'd recommend encapsulating the id in a structure rather than allocating
> sizeof(guint). It doesn't cost you much time now and if, in the future,
> you ever need to store any more information then the structure's already
> there.
> I noticed you already have a structure struct hci_dbus_data which
> contains just an integer. Did you mean to use that here or are they
> logically separate types (that just happen to be the same at the
> moment)?

The watch code is logically completely separate from the code that uses
the hci_dbus_data struct (the "id" variable refers to a GIOChannel id
while in the hci_dbus_data struct id refers to the device id). It is
also much simpler than the code that uses hci_dbus_data and I don't
think that anything besides the GIOChannel id needs to be passed to the
remove_watch function.

Johan


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 19:47:16

by Steven Singer

[permalink] [raw]
Subject: Re: [Bluez-devel] Re: hcid patch (remote name and connections)

Claudio Takahasi wrote:
> + if (!(cl = malloc(10 * sizeof(*ci) + sizeof(*cl)))) {
...
> + cl->conn_num = 10;

*cough* magic number *cough*

- Steven
--


This message has been scanned for viruses by BlackSpider MailControl - http://www.blackspider.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 19:45:18

by Steven Singer

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Johan Hedberg wrote:
> + id = malloc(sizeof(guint));

I'd recommend encapsulating the id in a structure rather than allocating
sizeof(guint). It doesn't cost you much time now and if, in the future,
you ever need to store any more information then the structure's already
there.

I noticed you already have a structure struct hci_dbus_data which
contains just an integer. Did you mean to use that here or are they
logically separate types (that just happen to be the same at the
moment)?

- Steven
--


This message has been scanned for viruses by BlackSpider MailControl - http://www.blackspider.com


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 19:28:01

by Johan Hedberg

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

On Tue, Oct 11, 2005, Marcel Holtmann wrote:
> let's wait until Johan finished his patch to remove the bad pointer
> versus integer casting.

Here you go. The idea was to wait for Claudio's new patch to be applied
first, but I guess we can do it in this order also. Btw, I haven't
tested the patch too much, but at least it compiles without warnings and
seems to run also withough any problems. The patch also changes the
dbus_watch stuff to use dynamically allocated memory instead of the
integer-pointer cast it was doing earlier.

Johan


Attachments:
(No filename) (540.00 B)
dbus-struct.patch (9.11 kB)
Download all attachments

2005-10-11 18:01:33

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] hcid patch (remote name and connections)

Hi Claudio,

> I added the support to remote name and display connections.
> This patch is based on dbus.c revision 1.20

let's wait until Johan finished his patch to remove the bad pointer
versus integer casting.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2005-10-11 14:49:11

by Claudio Takahasi

[permalink] [raw]
Subject: [Bluez-devel] Re: hcid patch (remote name and connections)

Hi folks, Marcel,

I added the support to remote name and display connections.
This patch is based on dbus.c revision 1.20

Marcel, please ignore the blocked e-mail. I sent a big attachement. :)

There is a command line client to test the added services:
http://www.cin.ufpe.br/~ckt/bluez/bluez-hcid-dbus-client.tar.gz

Regards,
Claudio.

On 10/11/05, Claudio Takahasi <[email protected]> wrote:
>
> Hi folks/Marcel,
>
> I added the support to remote name and display connections.
> This patch is based on dbus.c revision 1.20
>
> There is a command line client attached to test the added services.
>
> Regards,
> Claudio
>
> --
> ---------------------------------------------------------
> Claudio Takahasi
> Nokia's Institute of Technology - INdT
> [email protected]
>
>


--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]


Attachments:
(No filename) (935.00 B)
(No filename) (1.59 kB)
rem_name_connections_01.patch (7.42 kB)
Download all attachments