[libvirt] RFC: Coding style: require alphabetical header file sorting
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 :|
"Daniel P. Berrange" <berrange@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
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
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 :|
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,
On 12/03/2012 05:45 PM, Daniel P. Berrange wrote: 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
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 :|
participants (4)
-
Daniel P. Berrange -
John Eckersberg -
Michal Privoznik -
Viktor Mihajlovski