Page 1 of 1

Include guard names are not safe.

Posted: Wed Oct 30, 2013 1:38 am
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++.

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 4:10 am
by masterfalcon
I believe a bug was filed for this very issue a week or so ago.

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 10:30 am
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.

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 12:35 pm
by scrawl
I already reported this a while ago here: https://ogre3d.atlassian.net/browse/OGRE-316

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 6:53 pm
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...

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 8:19 pm
by Faranwath
Any reason not to use #pragma once? It appears to be widely supported and solves any naming issue.

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 8:39 pm
by scrawl
pragma once was discussed before, and rejected. https://bitbucket.org/sinbad/ogre/pull- ... ragma-once

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 9:21 pm
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.

Re: Include guard names are not safe.

Posted: Wed Oct 30, 2013 9:30 pm
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.

Re: Include guard names are not safe.

Posted: Thu Oct 31, 2013 2:31 pm
by Faranwath
Ok, ok, I abide by such a decision :D