Menu ▾ ▴

#55 reserved identifier violation

trunk
closed-accepted
nobody
None
5
2011-02-15
2010-12-11
Markus Elfring
No

I suggest to try the search pattern "\<_(?:(?:_(.*))|([A-Z]+))" on source files. A couple of places will be found where names begin with two underscores or an underscore and an uppercase letter.

Examples:
- _ANSI_CONVERT_H_
http://frhed.svn.sourceforge.net/viewvc/frhed/trunk/FRHED/AnsiConvert.h?revision=901&view=markup

- _REGISTRY_H_
http://frhed.svn.sourceforge.net/viewvc/frhed/trunk/FRHED/Registry.h?view=markup&revision=919

This does not fit to the expected naming conventions of the C/C++ language standard.
https://www.securecoding.cert.org/confluence/display/seccode/DCL37-C.+Do+not+use+identifiers+that+are+reserved+for+the+implementation

Discussion

  • Tim Gerundt

    Tim Gerundt - 2010-12-12

    Thanks for the hint! Do you have maybe also the interest to send us patches for this? :)

     

  • Markus Elfring

    Markus Elfring - 2010-12-13

    How do you think about to make include guards unique?
    Would you like to integrate anything from the appended adjustment into your source code repository?

     

  • Markus Elfring

    Markus Elfring - 2010-12-13

    update suggestion

     
    3135192.diff.zip

  • Tim Gerundt

    Tim Gerundt - 2010-12-14

    I committed your patch to SVN (In Revision 921)...

    Thank you for the bug report and the fix.

     

  • Tim Gerundt

    Tim Gerundt - 2010-12-14
    • status: open --> closed-fixed
     

  • Tim Gerundt

    Tim Gerundt - 2010-12-14

    Reopen, since there are still some open points.

     

  • Tim Gerundt

    Tim Gerundt - 2010-12-14
    • status: closed-fixed --> open-accepted
     

  • Tim Gerundt

    Tim Gerundt - 2010-12-14

    _DEBUG seems to be from the C run-time library:
    http://msdn.microsoft.com/en-us/library/0b98s6w8\(v=VS.90).aspx

    The same thing seems for the _UNICODE constant.

    _AFX_MINREBUILD looks really like something for the MFC libary.

    Maybe we should create a list from all remains.

     

  • Kimmo Varis

    Kimmo Varis - 2010-12-14

    Unfortunately we are working on Microsoft compiler-specific environment and some standards just don't hold in here. One reason is baggage from old versions that must have been kept intact even in latest SDK/compiler versions. One example is _DEBUG and _UNICODE. If you remove _UNICODE you just break Unicode build fatally. _DEBUG is similar thing, MS libraries use it so we must use it.

    > _AFX_MINREBUILD
    That must be some copy&paste artifact from me messing with that resource file stuff.

     

  • Kimmo Varis

    Kimmo Varis - 2010-12-14
    • labels: 1166123 -->
     

  • Kimmo Varis

    Kimmo Varis - 2010-12-14

    What is the point?

     

  • Markus Elfring

    Markus Elfring - 2010-12-14

    Can the symbol "_DEBUG" be replaced by "NDEBUG"?

     

  • Kimmo Varis

    Kimmo Varis - 2010-12-14

    No.

    Then one would need to use negation with it which only makes defines harder to read and understand. For absolute zero gain.

    I.e.
    #ifndef NDEBUG has double negation
    versus
    #ifdef _DEBUG is obvious

    In former the reader must think "IF NOT DEBUG define is NOT DEFINED". Latter is simple "IF _DEBUG is defined.

     

  • Markus Elfring

    Markus Elfring - 2010-12-14

    I prefer the symbol "NDEBUG" because it is supported by the C standard.

     

  • Kimmo Varis

    Kimmo Varis - 2010-12-18

    C standard is not C++ standard. This is not standard-coding practice but we need to adapt to the environment we have (MS compiler, MS libraries etc.).

    So the
    > _AFX_MINREBUILD
    is only thing left to fix (_DEBUG and _UNICODE won't be fixed)?

     

  • Markus Elfring

    Markus Elfring - 2010-12-18

    > C standard is not C++ standard. This is not standard-coding practice but we
    > need to adapt to the environment we have (MS compiler, MS libraries etc.).

    How do you think about the wording on special handling for (leading) underscores in the section "17.6.3.3.2 Global names" of the proposed C++ standard specification?
    http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3126.pdf

    > is only thing left to fix (_DEBUG and _UNICODE won't be fixed)?

    It is a matter how far we would like to reduce the dependency on open issues that were introduced by the Microsoft compilation environment.

     

  • Kimmo Varis

    Kimmo Varis - 2010-12-18

    > > is only thing left to fix (_DEBUG and _UNICODE won't be fixed)?
    > It is a matter how far we would like to reduce the dependency on open
    > issues that were introduced by the Microsoft compilation environment.
    When Frhed can be compiled with other compilers then those are trivial to fix. Until that there is no point in wasting time for that.

     

  • Kimmo Varis

    Kimmo Varis - 2011-02-15
    • status: open-accepted --> closed-accepted
     

  • Kimmo Varis

    Kimmo Varis - 2011-02-15

    Closing.

    I just noticed (somehow missed it the one patch was applied). But I really hope this kind of crap doesn't go again into our version control:
    -#ifndef _ANSI_CONVERT_H_
    -#define _ANSI_CONVERT_H_
    +#ifndef FRHED_ANSI_CONVERT_H_04C718D4726843F4AFFCDE8B07EFAEEA
    +#define FRHED_ANSI_CONVERT_H_04C718D4726843F4AFFCDE8B07EFAEEA

    This is totally pointless change. Only obfuscating the code and meaning of the include guards. We don't have multiple files with same name. And if we had we don't even need to care until we get the compile problems.

    I know Visual Studio adds some kind of GUID to the include guard names but I've always hated that. Even if some kids think it is cool it doesn't mean it is needed or wanted.

    And, there are thousands of much bigger and more complex projects which don't need that kind of include guard name games. We don't either.

    Closing this item now. And closing for comments too.