Re: [Bluez-devel] [PATCH] New keytab storage

[Fredrik Noring <noring@nocrew.org>]

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