10:45 a.m.
Thus far we only have one rule about header files, #include <config.h>
must be first in all .c files. I'm wondering if it is time to introduce
some new rules
- All system headers must be grouped to preceed all local headers
- All system headers must be sorted within their group
- All local headers must be sorted within their group
This will require updating pretty much every single source and header
file in the tree. Of course it will need a new syntax check rule to
validate this too. Since fixing them is a serious amount of work,
I was wondering about people's opinions on this ?
The goal is just standardization to make code a little more readable.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:01 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> writes:
Thus far we only have one rule about header files, #include
<config.h>
must be first in all .c files. I'm wondering if it is time to introduce
some new rules
- All system headers must be grouped to preceed all local headers
- All system headers must be sorted within their group
- All local headers must be sorted within their group
This will require updating pretty much every single source and header
file in the tree. Of course it will need a new syntax check rule to
validate this too. Since fixing them is a serious amount of work,
I was wondering about people's opinions on this ?
The goal is just standardization to make code a little more readable.
Daniel
I've recently been trying to find my way around the codebase for the
first time, +1 to this. In fact just this morning I was reading the
hacking guidelines trying to find this, because I expected it to be
there.
John
2:59 a.m.
On 03.12.2012 17:45, Daniel P. Berrange wrote:
Thus far we only have one rule about header files, #include
<config.h>
must be first in all .c files. I'm wondering if it is time to introduce
some new rules
- All system headers must be grouped to preceed all local headers
- All system headers must be sorted within their group
- All local headers must be sorted within their group
This will require updating pretty much every single source and header
file in the tree. Of course it will need a new syntax check rule to
validate this too. Since fixing them is a serious amount of work,
I was wondering about people's opinions on this ?
The goal is just standardization to make code a little more readable.
Daniel
I feel indifferent about the last two. Personally alphabetical sorting
would make it in fact harder if one is including twin header files, e.g.
for stat(2) you need to include:
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
which makes sense to me to keep together and not break into 3 pieces.
But I understand it may be easier for somebody to have header files sorted.
ACK to the first rule though.
Michal
4:10 a.m.
On Tue, Dec 04, 2012 at 09:59:54AM +0100, Michal Privoznik wrote:
On 03.12.2012 17:45, Daniel P. Berrange wrote:
> Thus far we only have one rule about header files, #include <config.h>
> must be first in all .c files. I'm wondering if it is time to introduce
> some new rules
>
> - All system headers must be grouped to preceed all local headers
> - All system headers must be sorted within their group
> - All local headers must be sorted within their group
>
> This will require updating pretty much every single source and header
> file in the tree. Of course it will need a new syntax check rule to
> validate this too. Since fixing them is a serious amount of work,
> I was wondering about people's opinions on this ?
>
> The goal is just standardization to make code a little more readable.
>
> Daniel
>
I feel indifferent about the last two. Personally alphabetical sorting
would make it in fact harder if one is including twin header files, e.g.
for stat(2) you need to include:
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
That is really a bogus manpage description. These days, all system
header files are intended to be self-contained, so for 'stat' you
only need sys/stat.h. This is shown correctly in the POSIX specs
http://pubs.opengroup.org/onlinepubs/9699919799/functions/stat.html
Even if that were not the case, your example doesn't scale - for example,
what would you do if using another function required
#include <sys/stat.h>
#include <fcntl.h>
you can't satisfy grouping of both sets of headers now, so IMHO, alphabetical
is the only option left at that point, other than "random" which is what
we have now.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:28 a.m.
On 12/03/2012 05:45 PM, Daniel P. Berrange wrote:
Thus far we only have one rule about header files, #include
<config.h>
must be first in all .c files. I'm wondering if it is time to introduce
some new rules
- All system headers must be grouped to preceed all local headers
why not enforce
the current statement in the contributor guidelines:
<config.h>, system includes, conditional system includes, "internal.h",
local includes (is there a need for conditional local includes?).
- All system headers must be sorted within their group
- All local headers must be sorted within their group
as a header file is
expected to be self-contained this ought to be OK,
although there might special cases requiring different ordering
(config.h is an obvious one, no others come to mind at the moment).
This will require updating pretty much every single source and header
file in the tree. Of course it will need a new syntax check rule to
validate this too. Since fixing them is a serious amount of work,
syntax-check
should allow exceptions to be defined per my
comment above.
I was wondering about people's opinions on this ?
The goal is just standardization to make code a little more readable.
would be a
first good step. Maybe out of scope, but I would even
welcome a general source-file structuring rule set (e.g. all static
definitions first, then all externals vs a more topic-oriented grouping,
like all network related functions, then all disk related ones, etc...).
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
7:09 a.m.
On Mon, Dec 03, 2012 at 04:45:26PM +0000, Daniel P. Berrange wrote:
Thus far we only have one rule about header files, #include
<config.h>
must be first in all .c files. I'm wondering if it is time to introduce
some new rules
- All system headers must be grouped to preceed all local headers
- All system headers must be sorted within their group
- All local headers must be sorted within their group
This will require updating pretty much every single source and header
file in the tree. Of course it will need a new syntax check rule to
validate this too. Since fixing them is a serious amount of work,
I was wondering about people's opinions on this ?
The goal is just standardization to make code a little more readable.
FYI, looking at this, I think it is not worth doing until we have
renamed all the files in src/util to have a 'vir' prefix.
We should then probably require that util headers are included first,
then the driver specific headers, or something like that
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4372
days inactive
4373
days old
5 comments
4 participants
participants (4)
-
Daniel P. Berrange -
John Eckersberg -
Michal Privoznik -
Viktor Mihajlovski