You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Andrew Stitcher <as...@redhat.com> on 2007/01/15 15:28:23 UTC

Qpid C++ header file includes

I'm a little concerned by the format of the header file guards being
used by the Qpid C++ code.

They mostly look like:

	#ifndef _SessionHandlerFactoryImpl_
	#define _SessionHandlerFactoryImpl_

Now according to the C++ standard (I'll look up the standard if anyone
is interested) symbols that have a single "_" followed by an uppercase
letter are reserved for the implementer (That is usually the
compiler/library writer). It's not clear to me whether symbols includes
macro symbols or not, but it seems better not to use them. Any symbol
starting with "__" is also reserved.

I'd suggest that we adopt a standard for include header guards that
doesn't start with "_" at all, preferable one that includes path
elements as well, as there was a case last week where 2 headers ended up
with the same guard temporarily and caused an annoying build problem.

Once we agree something I'm happy to go away and change all the header
files.

Any thoughts?

Andrew





Re: Qpid C++ header file includes

Posted by Andrew Stitcher <as...@redhat.com>.
On Mon, 2007-01-15 at 10:05 -0500, Kim van der Riet wrote:
> ...
> 
> Do we in fact use single trailing underscores anywhere in the C++?

I think we do use a single trailing underscore to indicate "member
variable" in some classes in qpid. But then again in some classes we use
a single leading underscore and in some we use neither, this area is
also a bit of a mess, but at least it doesn't conflict with the C++
standard (AFAIK)

Andrew



Re: Qpid C++ header file includes

Posted by Kim van der Riet <ki...@redhat.com>.
On Mon, 2007-01-15 at 14:52 +0000, Andrew Stitcher wrote:
> On Mon, 2007-01-15 at 09:42 -0500, Kim van der Riet wrote:
> > On Mon, 2007-01-15 at 14:38 +0000, Gordon Sim wrote: 
> > > Prefix with the namespaces? e.g. qpid_broker_SessionHandlerFactoryImpl_?
> > 
> > Unless we can turn up a well-defined standard or pattern, I think this
> > is as good a suggestion as any.
> 
> I think there are two natural solutions -
> * Use the file system hierarchy
> * Use the namespace hierarchy
> 
> I think the namespace hierarchy gives a more robust solution so I'd
> agree with this solution. We also need to use a scheme so that normal
> variables can't accidentally get the same name, so putting something in
> uppercase might be a good idea say:
> 
> QPID_BROKER_SessionHandlerFactoryImpl
> 
> or maybe use a double underscore at the end as we don't use that
> anywhere else:
> 
> qpid_broker_SessionHandlerFactoryImpl__
> 
> Andrew

I prefer the latter because the namespace portion matches the actual
code namespace (case-wise) and because it looks neater - if code
aesthetics is any measure to use in these matters... :-)

Do we in fact use single trailing underscores anywhere in the C++?

Kim


Re: Qpid C++ header file includes

Posted by Alan Conway <ac...@redhat.com>.
On Mon, 2007-01-15 at 14:52 +0000, Andrew Stitcher wrote:
> On Mon, 2007-01-15 at 09:42 -0500, Kim van der Riet wrote:
> > On Mon, 2007-01-15 at 14:38 +0000, Gordon Sim wrote: 
> > > Prefix with the namespaces? e.g. qpid_broker_SessionHandlerFactoryImpl_?
> > 
> > Unless we can turn up a well-defined standard or pattern, I think this
> > is as good a suggestion as any.
> 
> I think there are two natural solutions -
> * Use the file system hierarchy
> * Use the namespace hierarchy
> 
> I think the namespace hierarchy gives a more robust solution so I'd
> agree with this solution. We also need to use a scheme so that normal
> variables can't accidentally get the same name, so putting something in
> uppercase might be a good idea say:
> 
> QPID_BROKER_SessionHandlerFactoryImpl
> 
> or maybe use a double underscore at the end as we don't use that
> anywhere else:
> 
> qpid_broker_SessionHandlerFactoryImpl__

Either works, I prefer upcasing but don't really care as long as its
consistent.

Cheers,
Alan.


Re: Qpid C++ header file includes

Posted by Andrew Stitcher <as...@redhat.com>.
On Mon, 2007-01-15 at 09:42 -0500, Kim van der Riet wrote:
> On Mon, 2007-01-15 at 14:38 +0000, Gordon Sim wrote: 
> > Prefix with the namespaces? e.g. qpid_broker_SessionHandlerFactoryImpl_?
> 
> Unless we can turn up a well-defined standard or pattern, I think this
> is as good a suggestion as any.

I think there are two natural solutions -
* Use the file system hierarchy
* Use the namespace hierarchy

I think the namespace hierarchy gives a more robust solution so I'd
agree with this solution. We also need to use a scheme so that normal
variables can't accidentally get the same name, so putting something in
uppercase might be a good idea say:

QPID_BROKER_SessionHandlerFactoryImpl

or maybe use a double underscore at the end as we don't use that
anywhere else:

qpid_broker_SessionHandlerFactoryImpl__

Andrew



Re: Qpid C++ header file includes

Posted by Kim van der Riet <ki...@redhat.com>.
On Mon, 2007-01-15 at 14:38 +0000, Gordon Sim wrote: 
> Prefix with the namespaces? e.g. qpid_broker_SessionHandlerFactoryImpl_?

Unless we can turn up a well-defined standard or pattern, I think this
is as good a suggestion as any.

Kim


Re: Qpid C++ header file includes

Posted by Gordon Sim <gs...@redhat.com>.
Andrew Stitcher wrote:
> I'm a little concerned by the format of the header file guards being
> used by the Qpid C++ code.
> 
> They mostly look like:
> 
> 	#ifndef _SessionHandlerFactoryImpl_
> 	#define _SessionHandlerFactoryImpl_
> 
> Now according to the C++ standard (I'll look up the standard if anyone
> is interested) symbols that have a single "_" followed by an uppercase
> letter are reserved for the implementer (That is usually the
> compiler/library writer). It's not clear to me whether symbols includes
> macro symbols or not, but it seems better not to use them. Any symbol
> starting with "__" is also reserved.
> 
> I'd suggest that we adopt a standard for include header guards that
> doesn't start with "_" at all, preferable one that includes path
> elements as well, as there was a case last week where 2 headers ended up
> with the same guard temporarily and caused an annoying build problem.
> 
> Once we agree something I'm happy to go away and change all the header
> files.
> 
> Any thoughts?
> 
> Andrew
> 

Prefix with the namespaces? e.g. qpid_broker_SessionHandlerFactoryImpl_?