All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [Bluez-devel] Patch for bluez-hcidump-1.5
@ 2002-12-08  4:17 Maksim Yevmenkin
  0 siblings, 0 replies; 7+ messages in thread
From: Maksim Yevmenkin @ 2002-12-08  4:17 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: BlueZ Mailing List

Hi Marcel,

> > 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.
>=20
> 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.

amen :)

> > 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).
>=20
> Ok, got it. Is it not better to use the defined variable dlen?

yes, you can use dlen. for i386 architecture it probably makes no
difference anyway :)=20

> > > The man page belongs to section 8 like all other *dump programs do. S=
ee
> > > for example irdadump, tcpdump, pppdump etc. I will look at the man pa=
ge
> > > code today.

[...]

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

amen :)

> > *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=20
> > much easier for me to port, because when i see something like
> >=20
> > #include <bluetooth/x.h>
> > #include <bluetooth/y.h>
> > #include <bluetooth/z.h>
> >=20
> > rather then
> >=20
> > #include "something.h"
> >=20
> > 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.
>=20
> 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?

the answer to your first question lies in the style(9) man page :)
it defines the order of #inlude's. <sys/*.h>, <machine/*.h> and
other <*/*.h> come first, then other headers in alphabetical order
(if possible). i personally got used to this order and think it
makes sense. first you have #incude's that could possibly be system
dependant and then then rest of the #include's. however, i can deal
with any order of #include's as long as everything is working :)

porting to FreeBSD is pretty much straight forward, i.e. mechanical
changes for some types, #define's and structure names. however,
i would really like to not port anything at all, i.e. use original
BlueZ source code. unfortunately it is not possible right now.
low level API for HCI and L2CAP are somewhat different. although my
FreeBSD Bluetooth code also has HCI and L2CAP sockets - the types,
structure names and API (ioctl's, setsockopt defines) are still
different.=20

i will try to make the first step and create hci.h and l2cap.h
that will contain only HCI and L2CAP types and defines - no system
specific code. the next step is to define API for Bluetooth sockets
(HCI and L2CAP), i.e. ioctl names, setsockopt defines etc. i also
propose to define common libBluetooth, libHCI (and probably libL2CAP)
API that will hide possible implementation differences. after we
do that all application just will use defined API and source code
will be 100% portable.

as for RFCOMM, SDP etc. then you guys will lead the parade :)
i'm currently using BlueZ code with the minimal changes. i'm just
hoping that SDP, RFCOMM etc. implementation won't become too
Linux specific :)

thanks,
max

^ permalink raw reply	[flat|nested] 7+ messages in thread
* RE: [Bluez-devel] Patch for bluez-hcidump-1.5
@ 2002-12-07 17:07 Maksim Yevmenkin
  2002-12-08  0:18 ` Marcel Holtmann
  0 siblings, 1 reply; 7+ messages in thread
From: Maksim Yevmenkin @ 2002-12-07 17:07 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: BlueZ Mailing List

Hi Marcel,

> > please find attached patch for the bluez-hcidump-1.5 package.
> > here is the quick summary of the changes
> >=20
> > 1) __uXXX types replaced with uXXX_t=20
> > 2) minor fix for l2cap parser
> > 3) minor fix for hcidump.1 man page
> > 4) get_unaligned stuff fix
> >=20
> > Note: after applying the patch you still need to move
> > hcidump.8 to hcidump.1
> >=20
> > i also re-arranged #include lines .c files and removed #include
> > lines from the headers. this way it much easier to port, because
> > it is clear which headers are used and what needs to be ported.
> > i would appreciate if any new code in BlueZ will follow this
> > simple rules.=20
> >=20
> > please review and if there are no objections commit.
>=20
> please don't put all your changes in one big patch. Such big things are
> hard to review. I have removed your changes to the man page and attached
> the resulting diff to this email. People with non Intel architectures
> please test it. BTW the best command for making diffs is "diff -urN".

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.

> What do mean with 2. Maybe I have missed it, but I don't see any fixes
> to the L2CAP parser.

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).

> 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*, b=
ecause 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.

> What did you have used as reference? I ask because we should
> keep the same syntax for all of our man pages. Is there a "best way to
> write a man page"?

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.

> I left the re-arrange of the #include lines in the patch. I don't see
> why this makes it easier, but I have no big experiences on porting stuff
> to other OS. Can you give us (or maybe only me) some explanations why
> this is good to have.

*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=20
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.

thanks,
max

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [Bluez-devel] Patch for bluez-hcidump-1.5
@ 2002-12-07  5:04 Maksim Yevmenkin
  2002-12-07 16:01 ` Marcel Holtmann
  0 siblings, 1 reply; 7+ messages in thread
From: Maksim Yevmenkin @ 2002-12-07  5:04 UTC (permalink / raw)
  To: bluez-devel

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

Dear BlueZ-Developers,

please find attached patch for the bluez-hcidump-1.5 package.
here is the quick summary of the changes

1) __uXXX types replaced with uXXX_t 
2) minor fix for l2cap parser
3) minor fix for hcidump.1 man page
4) get_unaligned stuff fix

Note: after applying the patch you still need to move
hcidump.8 to hcidump.1

i also re-arranged #include lines .c files and removed #include
lines from the headers. this way it much easier to port, because
it is clear which headers are used and what needs to be ported.
i would appreciate if any new code in BlueZ will follow this
simple rules. 

please review and if there are no objections commit.

thanks,
max


[-- Attachment #2: bluez-hcidump-1.5.diff.gz --]
[-- Type: application/x-gzip, Size: 9064 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2002-12-09 18:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <45258A4365C6B24A9832BFE224837D552B1270@sjdcex01.int.exodus .net>
2002-12-09 17:58 ` [Bluez-devel] Patch for bluez-hcidump-1.5 Max Krasnyansky
2002-12-09 18:37   ` Maksim Yevmenkin
2002-12-08  4:17 Maksim Yevmenkin
  -- strict thread matches above, loose matches on Subject: below --
2002-12-07 17:07 Maksim Yevmenkin
2002-12-08  0:18 ` Marcel Holtmann
2002-12-07  5:04 Maksim Yevmenkin
2002-12-07 16:01 ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.