cruppstahl / upscaledb

A very fast lightweight embedded database engine with a built-in query language.
https://upscaledb.com
Apache License 2.0
570 stars 71 forks source link

reserved identifier violation #34

Closed elfring closed 10 years ago

elfring commented 10 years ago

I would like to point out that identifiers like "HAM_HAMSTERDB_HPP__" and "HAM_TYPES_H__" do not fit to the expected naming convention of the C++ language standard. Would you like to adjust your selection for unique names?

cruppstahl commented 10 years ago

Please don't bother - i plan to rearrange the files for the next release. I'll add this to my TODO list. Thanks for bringing it to my attention.

2014-08-19 20:33 GMT+02:00 Markus Elfring notifications@github.com:

I would like to point out that identifiers like "HAM_HAMSTERDB_HPP https://github.com/cruppstahl/hamsterdb/blob/8f29d7932bbb404e69ec49f273d18f3ed39661dc/include/ham/hamsterdb.hpp#L31" and "HAM_TYPES_H https://github.com/cruppstahl/hamsterdb/blob/0b4725db03b03ec9cec37431d92082ebd44eb91d/include/ham/types.h#L24" do not fit https://www.securecoding.cert.org/confluence/display/cplusplus/DCL32-CPP.+Do+not+declare+or+define+a+reserved+identifier#DCL32-CPP.Donotdeclareordefineareservedidentifier-NoncompliantCodeExample%28HeaderGuard%29 to the expected naming convention of the C++ language standard. Would you like to adjust your selection for unique names?

— Reply to this email directly or view it on GitHub https://github.com/cruppstahl/hamsterdb/issues/34.

elfring commented 10 years ago

Why did you close this issue without an explanation?

cruppstahl commented 10 years ago

I have renamed the reserved identifiers.

elfring commented 10 years ago

When will this change be published?

cruppstahl commented 10 years ago

It's distributed over several changes, because the whole directory structure has been modified heavily. In that process i also fixed the identifiers.

You can checkout the "v2" branch to get the new code. It's stable, but refactoring is still ongoing (as always...).

https://github.com/cruppstahl/hamsterdb/tree/v2/src

2014-09-11 12:40 GMT+02:00 Markus Elfring notifications@github.com:

When will this change be published?

— Reply to this email directly or view it on GitHub https://github.com/cruppstahl/hamsterdb/issues/34#issuecomment-55247218.

elfring commented 10 years ago

Are any header files still shared from a common include directory after your source file restructuring?

cruppstahl commented 10 years ago

The public API headers are still in /include/ham, but the internal header files are now spread over all the directories in /src.

The objective of the separation was not only to get a better overview of the files, but also to have more visibility regarding the separation of layers in the code. (See https://github.com/cruppstahl/hamsterdb/blob/v2/documentation/source/structure.txt). It's an experiment, but so far i find it helpful.

2014-09-11 12:53 GMT+02:00 Markus Elfring notifications@github.com:

Are any header files still shared from a common include directory after your source file restructuring?

— Reply to this email directly or view it on GitHub https://github.com/cruppstahl/hamsterdb/issues/34#issuecomment-55248213.

elfring commented 10 years ago

Would you like to make "the API header files" also standard-compliant?

cruppstahl commented 10 years ago

absolutely, thanks for the pointer.

2014-09-11 13:13 GMT+02:00 Markus Elfring notifications@github.com:

Would you like to make "the API header files https://github.com/cruppstahl/hamsterdb/blob/0c01e3ad84b24a08810099d38245adabb4e45442/include/ham/hamsterdb.h#L107" also standard-compliant http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier#answer-228797 ?

— Reply to this email directly or view it on GitHub https://github.com/cruppstahl/hamsterdb/issues/34#issuecomment-55249927.

elfring commented 10 years ago

Thanks for your acceptance of such a source code clean-up. It seems that there are a few update candidates left over for renaming.

elfring commented 10 years ago

Thanks for your source code improvement.

Does any header file from the subdirectory "tools" belong also to an application programming interface?

cruppstahl commented 10 years ago

Yes, and they're covered by this commit: 33c5ba3407ab1491890eb15b3155f2121263b1ba

Once again thanks for your help.

2014-09-13 7:27 GMT+02:00 Markus Elfring notifications@github.com:

Thanks for your source code improvement.

Does any header file from the subdirectory "tools" belong also to an application programming interface?

— Reply to this email directly or view it on GitHub https://github.com/cruppstahl/hamsterdb/issues/34#issuecomment-55482463.