Return-Path: Subject: Re: [Bluez-devel] [PATCH] New keytab storage From: Fredrik Noring To: Marcel Holtmann Cc: BlueZ Mailing List In-Reply-To: <1075509170.3594.36.camel@pegasus> References: <1075506991.14644.114.camel@akka.yeti.nocrew.org> <1075509170.3594.36.camel@pegasus> Content-Type: text/plain; charset=iso-8859-1 Message-Id: <1075510549.14644.135.camel@akka.yeti.nocrew.org> Mime-Version: 1.0 Date: Sat, 31 Jan 2004 01:55:49 +0100 List-ID: 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