Include guard names are not safe.

Minor issues with the Ogre API that can be trivial to fix
Post Reply
User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
x 56
Contact:

Include guard names are not safe.

Post by Klaim »

I just discovered that most (not all) headers in Ogre (1.9x) have their include guards with no library-name-prefix. For example:

Code: Select all

#ifndef __RenderWindow_H__
#define __RenderWindow_H__

Code: Select all

#ifndef _Archive_H__
#define _Archive_H__

Code: Select all

#ifndef __Animation_H__
#define __Animation_H__
etc...
OgreAny.h and some others don't have this issue.

Suggestion: make sure that all the include guards have the OGRE prefix. Also, don't use __ as prefix, it's reserved in C++.
User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
x 126
Contact:

Re: Include guard names are not safe.

Post by masterfalcon »

I believe a bug was filed for this very issue a week or so ago.
User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
x 56
Contact:

Re: Include guard names are not safe.

Post by Klaim »

I don't think I'll have time this week to submit a patch, but feel free to ask me later if you can't find anyone with time to do it when you're in the sprint to release Ogre 1.10, I'll be glad to help.
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: Include guard names are not safe.

Post by scrawl »

I already reported this a while ago here: https://ogre3d.atlassian.net/browse/OGRE-316
User avatar
lingfors
Hobgoblin
Posts: 525
Joined: Mon Apr 02, 2007 12:18 am
Location: Sweden
x 79

Re: Include guard names are not safe.

Post by lingfors »

Also, someone should fix the indentation... It seems currently it's an anarchistic mix between tabs and spaces, which means that anyone with tabs other than 4 get a horrible, horrible mess trying to read the Ogre source...
User avatar
Faranwath
Halfling
Posts: 93
Joined: Mon Jul 09, 2012 2:19 pm
Location: Cuba
x 7

Re: Include guard names are not safe.

Post by Faranwath »

Any reason not to use #pragma once? It appears to be widely supported and solves any naming issue.
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: Include guard names are not safe.

Post by scrawl »

pragma once was discussed before, and rejected. https://bitbucket.org/sinbad/ogre/pull- ... ragma-once
User avatar
Faranwath
Halfling
Posts: 93
Joined: Mon Jul 09, 2012 2:19 pm
Location: Cuba
x 7

Re: Include guard names are not safe.

Post by Faranwath »

As far as I can tell, no harm should come from this today. Granted, the dev team wants Ogre to build on every platform, but sometimes true standard-conforming code isn't the better. Include guards are, from my opinion, a good example of this.
User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
x 56
Contact:

Re: Include guard names are not safe.

Post by Klaim »

Unfortunately the lack of guarantee from the standard makes it a potential risk when porting for new plateforms with compilers that don't have (unprobable) pragma once or implement it in a different way than most compilers (more probable).

So even if for the sake of simplicity frankly we should do it, without guarantees from the standard, it opens a hole in the portability issues wall.
User avatar
Faranwath
Halfling
Posts: 93
Joined: Mon Jul 09, 2012 2:19 pm
Location: Cuba
x 7

Re: Include guard names are not safe.

Post by Faranwath »

Ok, ok, I abide by such a decision :D
Post Reply