RE: [Bluez-devel] Patch for bluez-hcidump-1.5

[Marcel Holtmann <marcel@rvs.uni-bielefeld.de>]

Hi Maksim,

> normally i would agree with you, but not in this case :) the changes
> are mostly mechanical, i.e s/__u8/uint8_t/g. this should be very easy
> to review. i did not change logic or code structure at all. however,
> if people think it is hard to review i will split it.

I like to have this mechanical one as a seperate patch so we can take a
quick look at it and apply them to CVS. If you have much mechanical
stuff in it is hard to find the other ones, especially if they are only
oneliner.

And don't diff Makefile.in, Makefile or configure. They are all
autogenerated.

> in the parser/l2cap.c file there is a function called parse_l2cap().
> after signal l2cap packet gets processed frm->len and frm->ptr
> are adjusted by hdr->len. i think it should be btohs(hdr->len).

Ok, got it. Is it not better to use the defined variable dlen?

> > The man page belongs to section 8 like all other *dump programs do. See
> > for example irdadump, tcpdump, pppdump etc. I will look at the man page
> > code today.
> 
> hmmm...
> 
> man 1 intro
> on Linux - Introduction to user commands
> on *BSD  - Introduction to general commands (tools and utilities)
> on Solaris - Introduction to commands and application programs
> 
> man 8 intro
> on Linux - Introduction to administration and privileged commands
> on *BSD  - Introduction to system maintainance and opearation commands
> on Solaris  - no man page?
> 
> tcpdump has .1 man page on *BSD, snoop (similar to tcpdump) has .1M
> man page on Solaris. tcpdump has .8 man page on Linux - really *strange*, because tcpdump does not even come standard with Linux (well at least
> in my case :). i had to install tcpdump RPM.
> 
> IMO tcpdump, pppdump, hcidump are hardly an "administration" commands,
> however i admit they may be "privileged". if Linux has chosen to
> put man pages for these tools in .8 section then it is fine. just
> revert Makefile.{im|am} and change section in the hcidump.8 file.

Let's have hcidump.8 because hcidump needs root privilige to work as
expected.

> man mdoc(7) on *BSD systems gives you general guidelines and page
> template. man mdoc(7) on Linux gives you a little bit less information,
> but still a good place to start.

Thanks, I will take a look at it.

> *BSD systems have style(9) man page. this page describes the
> coding style guidelines for *BSD kernel. i found these guidelines
> very useful and try to follow them. i *personally* try not to put
> #include lines in the header files. any dependencies should be
> indicated elsewhere. this comes from my experince with the couple
> of big projects where is was a pure nightmare to track down some
> minor change in the distant header file. another reason is that it 
> much easier for me to port, because when i see something like
> 
> #include <bluetooth/x.h>
> #include <bluetooth/y.h>
> #include <bluetooth/z.h>
> 
> rather then
> 
> #include "something.h"
> 
> i know exactly how to translate BlueZ headers into FreeBSD
> headers. i admin that it will not matter any more as soon as
> both systems will have common headers.

I agree with you that header files should avoid to include other header
files. But my question was, why you did the reorder of header files like
you have done in the patch. I always start with <stdio.h> and following.
Then take the <sys/*.h> and after that comes <bluetooth/*.h> as needed.
And I think this is a good order and I can't get the advantage of
changing it. What do you really need to make it easy to port them to
FreeBSD?

Regards

Marcel




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel