2004-01-30 23:56:31

by Fredrik Noring

[permalink] [raw]
Subject: [Bluez-devel] [PATCH] New keytab storage

Hi

Attached patch implements link key storage in /etc/bluetooth/keytab and
/etc/bluetooth/keytab.shadow. The old /etc/bluetooth/link_key file is
converted automatically.

I think a config option to change the default location would be useful.
This is not implemented yet. ("man" pages for hcid and hcid.conf is
badly needed btw.)

hcid is a bit more dynamic now so dmalloc and similar memory debugging
tools would be interesting to run. :)

I'd happy if anybody would like to test it.

Fredrik


Attachments:
hcid-keytab.patch.bz2 (10.86 kB)

2004-01-31 18:15:21

by Fredrik Noring

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

l?r 2004-01-31 klockan 19.01 skrev Marcel Holtmann:
> I think you get the idea.

The idea sounds excellent.

Fredrik

2004-01-31 18:01:28

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Fredrik,

> > But I think we should discuss some design goals of the new device manager.
>
> Great idea. What's your plan for it?

one of my big goals is to have a library with a much improved API. This
means that all HCI related functions use the hci_* prefix and follow the
naming scheme from the HCI part of the Bluetooth specification. But the
HCI part should not be used for common applications. These applications
should use bt_* functions that offer an enhanced functionality. For
example setting the local name. The hci_* part only send the HCI
command, while the bt_* stuff also stores the name in a database so it
can be restored after reboot. I think you get the idea.

> > I don't see what kind of mess in "glib-ectomy" you mean, but go ahead
> > and send a patch for it.
>
> Some bads, I think:
>
> - The API can be improved
> - Tons of ugly (even in opionon) typedef:s
> - No style

in the early days the hcid depends on glib and this code is a small
compat layer to be free from the glib. I wrote a similar replacement for
utils2. May you want to take a look at the helper library.

Regards

Marcel




-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2004-01-31 16:03:32

by Fredrik Noring

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

l?r 2004-01-31 klockan 16.52 skrev Marcel Holtmann:
> But I think we should discuss some design goals of the new device manager.

Great idea. What's your plan for it?

> I don't see what kind of mess in "glib-ectomy" you mean, but go ahead
> and send a patch for it.

Some bads, I think:

- The API can be improved
- Tons of ugly (even in opionon) typedef:s
- No style

Fredrik

2004-01-31 15:52:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Fredrik,

> > the compat directories contains the old code until all parts are
> > replaced. This is why it is called compat. Don't focus on it.
>
> Since the "new" hcid was lost and there is no replacement, I'll work
> on the old code. I think it's very useful. I'm currently cleaning up
> the mess in "glib-ectomy" and preparing it for a DBus connection.

it is not very hard to replace this code, but I am a little bit busy and
the CVS is dead at the moment. But I think we should discuss some design
goals of the new device manager.

I don't see what kind of mess in "glib-ectomy" you mean, but go ahead
and send a patch for it. The D-Bus support is one of the things I
definitively want to have in the next release, so every patch would help
here. Please keep both separate, because small patches are easier to
review.

Regards

Marcel




-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2004-01-31 15:40:28

by Fredrik Noring

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Marcel

l?r 2004-01-31 klockan 16.23 skrev Marcel Holtmann:
> the compat directories contains the old code until all parts are
> replaced. This is why it is called compat. Don't focus on it.

Since the "new" hcid was lost and there is no replacement, I'll work
on the old code. I think it's very useful. I'm currently cleaning up
the mess in "glib-ectomy" and preparing it for a DBus connection.

Fredrik

2004-01-31 15:23:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Fredrik,

> I haven't looked closely at libs2 but the hcid parts in utils2 appears
> to be a copy of the original utils, except the files are renamed and put
> in a "compat" directory with lots of other utilities. Why is that?
>
> A "hcid" directory (as in the old "libs") would be better I think.

the compat directories contains the old code until all parts are
replaced. This is why it is called compat. Don't focus on it.

> Maybe this a FAQ but I only get this:
>
> $ cvs -d:pserver:[email protected]:/cvsroot/bluez login
> Logging in to :pserver:[email protected]:2401/cvsroot/bluez
> CVS password:
> cvs [login aborted]: end of file from server (consult above messages if any)
> $
>
> Any hints, please?

We ran into a SF problem, again. Even my latest update failed and now
the developer access is also blocked. We have to wait until they solved
it.

Regards

Marcel




-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2004-01-31 10:47:04

by Fredrik Noring

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Marcel

l?r 2004-01-31 klockan 01.32 skrev Marcel Holtmann:
> Follow the coding style.

Attached patch contains a bunch of cleanups.

file.c | 41 ++++++----------
file.h | 18 +++----
hcid.h | 12 ++--
keytab.c | 154 ++++++++++++++++++++++++++++++-------------------------------
keytab.h | 18 +++----
lib.h | 4 -
security.c | 11 ++--
7 files changed, 124 insertions(+), 134 deletions(-)

Fredrik


Attachments:
hcid-cleanups.patch (15.31 kB)

2004-01-31 09:27:14

by Fredrik Noring

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Marcel

l?r 2004-01-31 klockan 02.18 skrev Marcel Holtmann:
> However you should talk a look at libs2 and utils2 from CVS.

I haven't looked closely at libs2 but the hcid parts in utils2 appears
to be a copy of the original utils, except the files are renamed and put
in a "compat" directory with lots of other utilities. Why is that?

A "hcid" directory (as in the old "libs") would be better I think.

> Generate the diffs againt the CVS sources.

Maybe this a FAQ but I only get this:

$ cvs -d:pserver:[email protected]:/cvsroot/bluez login
Logging in to :pserver:[email protected]:2401/cvsroot/bluez
CVS password:
cvs [login aborted]: end of file from server (consult above messages if any)
$

Any hints, please?

> Hope you get my point.

Yes, thanks.

Fredrik

2004-01-31 01:18:43

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Fredrik,

> Can I ask where the new code is?

I started already coding the new device manager, but accidentally I
deleted that code from my harddisk.

However you should talk a look at libs2 and utils2 from CVS.

> I'd be happy to skip them. The original source (bluez-utils-2.4.tar.gz)
> contains these files for some reason though.

Generate the diffs againt the CVS sources.

> The main reason for this typedef is that the original code did things
> like this:
>
> unint8_t link_key_t[16];
> ...
> memcpy(key0, key1, 16);
>
> which, I think, is a lot worse than:
>
> memcpy(key0, key1, sizeof(link_key_t));
>
> Don't you agree? (An alternative is to use #define LINK_KEY_SIZE 16
> instead of course.)

I am not sure how to handle this link key issue in general, but that was
not the argument. See below.

> > than the *_t version must match the structure and not define something
> > complete different.
>
> What do you mean?

If you typedef something and it ends with "_t" you should avoid to have
a struct with the same prefix, but different meaning. This example is
ok, but it is not good coding style (typedefs are ugly).

struct abc {
int a;
int b;
int c;
};

typedef struct abc abc_t;

Hope you get my point.

Regards

Marcel




-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2004-01-31 00:55:49

by Fredrik Noring

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Marcel,

l?r 2004-01-31 klockan 01.32 skrev Marcel Holtmann:
> I haven't reviewed the complete patch and at the moment and I am not
> convinced in doing such a big change to the old code. But I have some
> general comments.

Can I ask where the new code is?

> Please send every patch as plain text and not compressed, because this
> makes it easy to take a first look at it within the email client. If you
> got oversize reports from the mailing list manager, don't worry I will
> approve the email by hand.

OK, I'll do that.

> If you do such a big change show us the output of diffstat so everybody
> can see what files are going to be changed.

Sure.

> Don't include files that are generated by autoconf/automake. In your
> case this is Makefile.in. diffstat or lsdiff helps here to check the
> patch for unwanted files.

I'd be happy to skip them. The original source (bluez-utils-2.4.tar.gz)
contains these files for some reason though.

> Follow the coding style. Read the paper from Greg Kroah-Hartman. It is
> very good.

OK.

> Typedefs are ugly, but if you use them,

The main reason for this typedef is that the original code did things
like this:

unint8_t link_key_t[16];
...
memcpy(key0, key1, 16);

which, I think, is a lot worse than:

memcpy(key0, key1, sizeof(link_key_t));

Don't you agree? (An alternative is to use #define LINK_KEY_SIZE 16
instead of course.)

> than the *_t version must match the structure and not define something
> complete different.

What do you mean?

Many thanks for your comments,
Fredrik

2004-01-31 00:32:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] [PATCH] New keytab storage

Hi Fredrik,

> Attached patch implements link key storage in /etc/bluetooth/keytab and
> /etc/bluetooth/keytab.shadow. The old /etc/bluetooth/link_key file is
> converted automatically.

I haven't reviewed the complete patch and at the moment and I am not
convinced in doing such a big change to the old code. But I have some
general comments.

Please send every patch as plain text and not compressed, because this
makes it easy to take a first look at it within the email client. If you
got oversize reports from the mailing list manager, don't worry I will
approve the email by hand.

If you do such a big change show us the output of diffstat so everybody
can see what files are going to be changed.

Makefile.am | 4
Makefile.in | 548 ++++++++++++++++++++++++++++++++++++------------------------
file.c | 92 ++++++++++
file.h | 18 +
hcid.h | 21 +-
keytab.c | 459 ++++++++++++++++++++++++++++++++++++++++++++++++++
keytab.h | 14 +
lib.h | 4
main.c | 2
security.c | 108 ++---------
10 files changed, 952 insertions(+), 318 deletions(-)

Don't include files that are generated by autoconf/automake. In your
case this is Makefile.in. diffstat or lsdiff helps here to check the
patch for unwanted files.

Follow the coding style. Read the paper from Greg Kroah-Hartman. It is
very good. Here are my notes:

The comment should be above the function declaration. Some goes for any
other statement.

static int parse_hex_digit(char c)
/* Returns binary 0-15 for hex '0'-'a' and '0'-'A'.
* Returns -1 for invalid hex digits. */
{

The parenthese must be behind the "if", "for" and "while" statement.

fd = open(filename, O_RDONLY);
if (fd < 0)
{

Include a whitespace after "if", "for", "while" etc.

for(;;)
{

Typedefs are ugly, but if you use them, than the *_t version must match
the structure and not define something complete different.

typedef uint8_t link_key_t[16];

struct link_key {

Regards

Marcel




-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel