Bluez exceptions refactoring updated patch
PATCH 1/1 Bluez exceptions refactoring updated patch
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
Marcel Holtmann
maintainerHi Fabien,
Quoted 5 lines
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.
Quoted 7 lines
Changes compared to previous version:
- renamed the common .h and .c aserror.handerror.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 [email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Hi Marcel,
Please some comments below...
Quoted 2 lines
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 :-) )
Quoted 4 lines
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 fileinclude/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
Marcel Holtmann
maintainerHi Fabien,
Quoted 7 lines
Quoted 2 lines
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.
Quoted 9 lines
Quoted 3 lines
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.
Quoted 5 lines
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 fileinclude/linux/usb.hfor 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 [email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Hi Marcel,
Quoted 4 lines
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.
Quoted 15 lines
Quoted 9 lines
Quoted 4 lines
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.
Quoted 5 lines
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 fileinclude/linux/usb.hfor instance).
Quoted 6 lines
And check
drivers/usb/core/urb.cfor 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Marcel Holtmann
maintainerHi Fabien,
Quoted 7 lines
Quoted 3 lines
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_bluetoothandctl_bluetoothfiles, 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.
Quoted 1 line
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Hi Fabien,
Quoted 5 lines · Fabien Chevalier
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.
Quoted 7 lines
Changes compared to previous version:
- renamed the common .h and .c aserror.handerror.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