Subject: Re: [Bluez-devel] [PATCH] New keytab storage
From: Marcel Holtmann <marcel@holtmann.org>
To: Fredrik Noring <noring@nocrew.org>
Cc: BlueZ Mailing List <bluez-devel@lists.sourceforge.net>
In-Reply-To: <1075506991.14644.114.camel@akka.yeti.nocrew.org>
References: <1075506991.14644.114.camel@akka.yeti.nocrew.org>
Content-Type: text/plain
Message-Id: <1075509170.3594.36.camel@pegasus>
Mime-Version: 1.0
Sender: bluez-devel-admin@lists.sourceforge.net
Errors-To: bluez-devel-admin@lists.sourceforge.net
Date: Sat, 31 Jan 2004 01:32:50 +0100

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
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel