[bind10-dev] coding guidelines

[Francis Dupont]

> > > 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?
> 
> What system dependencies?

=> typically the socket includes or things like unistd.h (which BTW
doesn't fit well on WIN32 :-)

> The dependencies to compile the header, yes.

=> "to compile the header" is not really defined. I propose to consider
what happens with an empty/unrelated .cc body.

> The ones needed for implementation only, no. So, if I take the
> example of lock.{h,cc}, the pthreads.h goes to the .cc, because
> there's nothing in the public interface of the wrapper that'd need
> it. If there was a method returning the mutex_t, then pthreads.h
> would go to the header file.

=> this doesn't fit what I can read in the master src/lib/util/threads
but I think I caught your idea: the dependencies are strictly about
required defines for the header file.

> And I guess config.h should go first whenever needed. But I'm not sure about
> that, we don't come into contact with this one too often O:-).

=> config.h is where to put system dependent flags. There are a few in
most Unixes but a lot for WIN32 (which typically includes windows.h).
So the question is whether config.h should go to header files or
not. According to your argument it should be the case as soon as
something requiring it is included too in a header file.

> > > 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)
>
> I'm not sure if we understand each other. What I'm saying is, if
> you're referring to "local" header (lock.cc includes lock.h or
> thread.cc includes lock.h, all of them living in one directory),
> then use " " without any path.  If you're using lock.h in the auth
> server, then use < >, because it's not local file.

=> the concern I have about " " is it doesn't say to ignore the -I
paths, it just changes the order to search is done. More, this order
change is compiler dependent, e.g., gcc and msvc don't use the same!
And the C11 standard says if the " " specific search fails the include
must be interpreted as with < >, so there is no way to "protect" a
" " include.

> I _think_ it is OK to refer to a subdirectory of local directory by
>  " ", so you could do things like "rdata/generic/cname_5.h" (if
>  these were ever included directly) from files in dns directory. But
>  as <dns/rdata/generic/cname_5.h> from elsewhere.

=> I missed the (uncommon in bind 10) subdirectory case but
I agree it is a legitim exception to the no '/'.

> Anyway, what I say is:
>  - Use < > when including by search path.
>  - " " if it is relative to the file into which you include.

=> relative is not the right term: IMHO you want to mean something stricter
(e.g., no ../xxx/yyy)

>  - Avoid search path if reasonably possible.
> 
> Which basically concludes to what I said above. But " " is not evil,
> it has its place.

=> anyway there should be something in the guidelines and this must be
enforced at the first occasion. For instance I am porting bind10 to WIN32
(cf trac2351) and I can change in
src/lib/exceptions/exceptions.cc

#include <exceptions/exceptions.h>

into

#include "exceptions.h"

as an application of your proposed rule.
And same for src/lib/exceptions/tests/exceptions_unittest.cc?

Regards

Francis Dupont <fdupont at isc.org>

PS: I'd like to get more opinions!