[bind10-dev] coding guidelines

[Francis Dupont]

> On Sun, Oct 14, 2012 at 10:30:26AM +0000, Francis Dupont wrote:
> >  - Close all file and socket objects: running tests (aka make check) on
> >   some systems show it is not enforced in tests. Perhaps it doesn't
> >   matter, perhaps it is too easy to leak a file or socket descriptor?
> >   (PS: the messages are from a closing python in debug mode as far as
> >    I can remember)
> 
> I think some tests are older than the guidelines. We didn't invest
> the effort to fix all tests, because some were just too much work
> with nearly no benefit.

=> IMHO the "don't" should be "at lowest priority"...

> >  - Ordering Include Files: the proposed order is the opposite of
> >   the usual one but IMHO it is from the statement about header
> >   files themselves.
> >   BTW how this applies to config.h?
> 
> I'm not sure about the generally usual one, but I'm sure this is not
> the first project with Local headers first. We decided this is
> better, because then we can test the headers have no hidden
> dependencies.

=> so you confirm the system dependencies should go in header files.
And about config.h? where to put it?

> >  - Ordering Include Files: this section MUST decide about <> vs "" and
> >   IMHO suggest full relative paths (i.e., dns/message.h even in the
> >   src/lib/dns directory): the idea is to have *only* -I$(xxx)/src/lib
> >   in the Makefile. BTW this avoids ambiguity with 2 headers with the
> >   same name.
> 

> I'd strongly disagree here. I'm OK with full relative paths in < >.
> But the " " includes have their place. The " " ones look into
> the local directory first.

=> can I infer from your argument?
 - " " should always a file name without a path (i.e., no '/' in it)
 - " " doesn't make sense for a library in general
 - " " should never refer to something in another directory (i.e.,
  in such a case < > should be used)

> If you write:
> 
>   #include <dns/name.h>
> 
> you may get unexpected results if something like
> -I/usr/lib/bind10-other-version gets at the front of your include
> path.

=> I disagree: results are expected and wrong. If I apply your argument
to library paths, it says it must include only "."...

> If you write:
>
> 
>   #include "name.h"
> 
> from another file in the dns directory, you're sure the correct
> (local) name.h gets picked. So I tend to prefer " " for local
> files. Also, it makes it clear this is a local/related header file,
> not an external library one.

=> I deeply disagree: this is not the right way to solve the issue,
one could rely on the include path (BTW this is a reason to keep it
short :-).

> Also, writing things like
> 
>   #include "dns/name.h"
> 
> is IMO completely wrong, since the " " one is meant to be relative
> to local directory.

=> for this point we agree.

Regards

Francis Dupont <fdupont at isc.org>