News:

When registered with our forums, feel free to send a "here I am" post here to differ human beings from SPAM bots.

Main Menu

static variables in cbplugin.h

Started by ollydbg, December 27, 2011, 02:47:58 AM

Previous topic - Next topic

ollydbg

I just found a potential bug:
Here, in cbplugin.h, there are code like:

// Define basic groups for plugins' configuration.
static const int cgCompiler         = 0x01; ///< Compiler related.
static const int cgDebugger         = 0x02; ///< Debugger related.
static const int cgEditor           = 0x04; ///< Editor related.
static const int cgCorePlugin       = 0x08; ///< One of the core plugins.
static const int cgContribPlugin    = 0x10; ///< One of the contrib plugins (or any third-party plugin for that matter).
static const int cgUnknown          = 0x20; ///< Unknown. This will be probably grouped with cgContribPlugin.


This will let all the translation unit which include the "cbplugin.h" have a own copy of those variables. Is that a bug?

Why not just use extern modifier, and define them in the cpp files???
Or just use enum?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

danselmi

They are defined as const. So a "copy" is the same constant value.
spell checker plugin: [url="http://developer.berlios.de/projects/spellchecker/"]http://developer.berlios.de/projects/spellchecker/[/url]
nassi shneiderman plugin: [url="http://developer.berlios.de/projects/nassiplugin"]http://developer.berlios.de/projects/nassiplugin[/url]

Folco

This is a linker optimisation, no ? "Constant merging". Is it guaranteed by the standard ?
Kernel Extremist - PedroM power ©

ollydbg

Quote from: danselmi on December 27, 2011, 12:03:06 PM
They are defined as const. So a "copy" is the same constant value.
Yes. But...Anyway, I do believe variable definitions in header file is not a good idea.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

MortenMacFly

Quote from: ollydbg on December 27, 2011, 02:49:21 PM
Yes. But...Anyway, I do believe variable definitions in header file is not a good idea.
static const can be seen as the c++ style of a #define. In the case you refer to it's also similar to an enum, so this is OK.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

oBFusCATed

Morten: do you know the reason why this is not an enum, but a number of static variables?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

thomas

#6
ISO 14882 3.5.3 states that names having namespace scope automatically have internal linkage if they are either const or static and not at the same time explicitly extern. Which means that the linkage is irrespective of the presence of static because of const. The rationale for that is given in Appendix C. The standard makes explicit mention of inclusion of headers in many compilation units.

As to why these values are not an enum, the likely reason is simply that the person who wrote that code (Yiannis) wrote it that way. There is no real advantage or disadvantage to either optimization-wise.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

MortenMacFly

Quote from: oBFusCATed on December 29, 2011, 01:35:41 PM
Morten: do you know the reason why this is not an enum, but a number of static variables?
Usually this is because if you compare the values against another one which is an INT, too you don't need to take care to cast the values to avoid compiler warnings. At least that's when I personally prefer static const instead of an enum.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

ollydbg

Quote from: MortenMacFly on December 29, 2011, 04:43:50 PM
Usually this is because if you compare the values against another one which is an INT, too you don't need to take care to cast the values to avoid compiler warnings.
I guess that if it was defined as a enum, then the user/client will define same enum type variables too, some kind of type consistent. :)

"Int type" basically does not have much specific information like enum type.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

MortenMacFly

Quote from: ollydbg on December 30, 2011, 01:07:56 AM
I guess that if it was defined as a enum, then the user/client will define same enum type variables too, some kind of type consistent. :)
I thought more in a direction to compare it against a value of a control or alike. There, you have no choice other than INT or SIZE_T or alike.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]