2007-11-25 21:32:11

by Fabien Chevalier

[permalink] [raw]
Subject: [PATCH] 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


Attachments:
exceptions-rework-global.diff.gz (23.84 kB)

2007-11-30 09:41:36

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2007-11-30 09:01:38

by Fabien Chevalier

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


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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2007-11-27 20:54:57

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

2007-11-26 16:12:13

by Fabien Chevalier

[permalink] [raw]
Subject: Re: [PATCH] Bluez exceptions refactoring updated patch

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

2007-11-26 14:12:12

by Johan Hedberg

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

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

2007-11-26 05:47:31

by Marcel Holtmann

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

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel