[PATCH] Bluez exceptions refactoring updated patch

[PATCH] Bluez exceptions refactoring updated patch

[Fabien Chevalier]

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

Hi all,

I finally managed to spend a few hours of my spare time on Bluez.

As a result i updated the patch that updates the common/ directory as 
well as the various services to share the exceptions definition.

Changes compared to previous version:
  - renamed the common .h and .c as error.h and error.c
  - completely removed various error.{c,h} from services directory
  - updated api documentation (dbus-api.txt + services api definition).

The patch is pretty big, if needed i can provided splitted patches for 
various directories.

Cheers,

Fabien

[-- Attachment #2: exceptions-rework-global.diff.gz --]
[-- Type: application/gzip, Size: 24417 bytes --]

Re: [Bluez-devel] [PATCH] Bluez exceptions refactoring updated patch

[Marcel Holtmann]

Hi Fabien,

> I finally managed to spend a few hours of my spare time on Bluez.
> 
> As a result i updated the patch that updates the common/ directory as 
> well as the various services to share the exceptions definition.
> 
> Changes compared to previous version:
>   - renamed the common .h and .c as error.h and error.c
>   - completely removed various error.{c,h} from services directory
>   - updated api documentation (dbus-api.txt + services api definition).
> 
> The patch is pretty big, if needed i can provided splitted patches for 
> various directories.

I think we should do it in a few steps to make it actually readable if
we ever have to track this down:

	- Create common/error.[ch]
	- Update dbus-api.txt
	- Remove error.[ch] from every service and use common/error.h
	  It should be one patch per service and also update its API
	  document

Two comments on your patch itself. The copyright on common/error.[ch] is
not only yours. Keep the original copyright of the files.

It is great that you added comments for each function, but they belong
inside the *.c files. The rule is that they should be placed where the
actual function body is defined.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [PATCH] Bluez exceptions refactoring updated patch

[Fabien Chevalier]

Hi Marcel,

Please some comments below...

> Two comments on your patch itself. The copyright on common/error.[ch] is
> not only yours. Keep the original copyright of the files.

Well, that file didn't exit beforehand, so as far as nobody else but me 
has touched it, what's the point in putting everybody's copyright in it?
(OK, this is not true anymore as Johan made some changes you requested 
to it :-) )

> 
> It is great that you added comments for each function, but they belong
> inside the *.c files. The rule is that they should be placed where the
> actual function body is defined.

Marcel, i don't know where from you got this idea, but i find it pretty 
dumb. The whole point in having the documentation in the header file is 
that as a user of a function you're not interested in how it works (the 
c file), but what it does, which is what the documentation is for.

I don't know of *any* project that has it's function level documentation 
in the C files.
But there are numerous counter-examples:
   - The Linux Kernel (have a look at the USB stack file 
include/linux/usb.h for instance).
   - glib
   - libsig++
   - Qt
   - STL
   - ... ==> Do i need to find more ?

So, is there any good reason we should do differently from everybody 
else ? :-)

Cheers,

Fabien

Re: [Bluez-devel] [PATCH] Bluez exceptions refactoring updated patch

[Marcel Holtmann]

Hi Fabien,

> > Two comments on your patch itself. The copyright on common/error.[ch] is
> > not only yours. Keep the original copyright of the files.
> 
> Well, that file didn't exit beforehand, so as far as nobody else but me 
> has touched it, what's the point in putting everybody's copyright in it?
> (OK, this is not true anymore as Johan made some changes you requested 
> to it :-) )

and the origin is from the other error.[ch] files. So the copyright
still prevails. Copying content from one file to another one doesn't
change the copyright.
 
> > It is great that you added comments for each function, but they belong
> > inside the *.c files. The rule is that they should be placed where the
> > actual function body is defined.
> 
> Marcel, i don't know where from you got this idea, but i find it pretty 
> dumb. The whole point in having the documentation in the header file is 
> that as a user of a function you're not interested in how it works (the 
> c file), but what it does, which is what the documentation is for.
> 
> I don't know of *any* project that has it's function level documentation 
> in the C files.
> But there are numerous counter-examples:
>    - The Linux Kernel (have a look at the USB stack file 
> include/linux/usb.h for instance).

And check drivers/usb/core/urb.c for another example. The documentation
comment are not for reading the *.h. They are for creating automated API
documentation. The that structs are document in the header file is
right, because they are defined there. The actual function are defined
in the *.c files and thus the comment belongs there.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Bluez exceptions refactoring updated patch

[Fabien Chevalier]

Hi Marcel,

> 
> and the origin is from the other error.[ch] files. So the copyright
> still prevails. Copying content from one file to another one doesn't
> change the copyright.

If this is the case i should have a couple of copyrights in the 
pcm_bluetooth and ctl_bluetooth files, as if i remember well they both 
started as a copy& past from plugz project ;-)
I will surely send a patch to fix that one of those days.

>  
>>> It is great that you added comments for each function, but they belong
>>> inside the *.c files. The rule is that they should be placed where the
>>> actual function body is defined.
>> Marcel, i don't know where from you got this idea, but i find it pretty 
>> dumb. The whole point in having the documentation in the header file is 
>> that as a user of a function you're not interested in how it works (the 
>> c file), but what it does, which is what the documentation is for.
>>
>> I don't know of *any* project that has it's function level documentation 
>> in the C files.
>> But there are numerous counter-examples:
>>    - The Linux Kernel (have a look at the USB stack file 
>> include/linux/usb.h for instance).
> 
> And check drivers/usb/core/urb.c for another example. The documentation
> comment are not for reading the *.h. They are for creating automated API
> documentation. The that structs are document in the header file is
> right, because they are defined there. The actual function are defined
> in the *.c files and thus the comment belongs there.
> 

If you wish to follow kernel documenting style, then so be it. We're 
clearly sitting in the minority side however ;-)

Cheers,

Fabien

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Bluez exceptions refactoring updated patch

[Marcel Holtmann]

Hi Fabien,

> > and the origin is from the other error.[ch] files. So the copyright
> > still prevails. Copying content from one file to another one doesn't
> > change the copyright.
> 
> If this is the case i should have a couple of copyrights in the 
> pcm_bluetooth and ctl_bluetooth files, as if i remember well they both 
> started as a copy& past from plugz project ;-)

in the beginning they didn't start out as copy&paste. I wrote them from
scratch to have them clean. Right now the situation might be different.

> I will surely send a patch to fix that one of those days.

Sure. Go ahead. I am trying to keep track of it, but the project grows
so big that I might simply miss things here and there. Especially with
all this outside CVS development.

Regards

Marcel



-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Bluez exceptions refactoring updated patch

[Johan Hedberg]

Hi Fabien,

On Sun, Nov 25, 2007, Fabien Chevalier wrote:
> I finally managed to spend a few hours of my spare time on Bluez.
>
> As a result i updated the patch that updates the common/ directory as well 
> as the various services to share the exceptions definition.
>
> Changes compared to previous version:
>  - renamed the common .h and .c as error.h and error.c
>  - completely removed various error.{c,h} from services directory
>  - updated api documentation (dbus-api.txt + services api definition).
>
> The patch is pretty big, if needed i can provided splitted patches for 
> various directories.

The patch is now fixed, split into parts and commited to the CVS as
Marcel suggested so no need to do further work on that part.

Johan