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
Contact:

Include guard names are not safe.

Post by Klaim » Wed Oct 30, 2013 1:38 am

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++.
0 x

User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
Contact:

Re: Include guard names are not safe.

Post by masterfalcon » Wed Oct 30, 2013 4:10 am

I believe a bug was filed for this very issue a week or so ago.
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Include guard names are not safe.

Post by Klaim » Wed Oct 30, 2013 10:30 am

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.
0 x

scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 2

Re: Include guard names are not safe.

Post by scrawl » Wed Oct 30, 2013 12:35 pm

I already reported this a while ago here: https://ogre3d.atlassian.net/browse/OGRE-316
0 x

User avatar
lingfors
Hobgoblin
Posts: 505
Joined: Mon Apr 02, 2007 12:18 am
Location: Sweden

Re: Include guard names are not safe.

Post by lingfors » Wed Oct 30, 2013 6:53 pm

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...
0 x

User avatar
Faranwath
Halfling
Posts: 93
Joined: Mon Jul 09, 2012 2:19 pm
Location: Cuba

Re: Include guard names are not safe.

Post by Faranwath » Wed Oct 30, 2013 8:19 pm

Any reason not to use #pragma once? It appears to be widely supported and solves any naming issue.
0 x

scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 2

Re: Include guard names are not safe.

Post by scrawl » Wed Oct 30, 2013 8:39 pm

pragma once was discussed before, and rejected. https://bitbucket.org/sinbad/ogre/pull- ... ragma-once
0 x

User avatar
Faranwath
Halfling
Posts: 93
Joined: Mon Jul 09, 2012 2:19 pm
Location: Cuba

Re: Include guard names are not safe.

Post by Faranwath » Wed Oct 30, 2013 9:21 pm

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.
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Include guard names are not safe.

Post by Klaim » Wed Oct 30, 2013 9:30 pm

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.
0 x

User avatar
Faranwath
Halfling
Posts: 93
Joined: Mon Jul 09, 2012 2:19 pm
Location: Cuba

Re: Include guard names are not safe.

Post by Faranwath » Thu Oct 31, 2013 2:31 pm

Ok, ok, I abide by such a decision :D
0 x

Post Reply