search

nasa / jeod

Other
26 stars 5 forks source link

reserved identifier violation #3

Closed elfring closed 11 months ago

elfring commented 1 year ago

I would like to point out that identifiers like “_DEG2RAD” and “_ORB_ELEM_MULTI_VER_H_” do eventually not fit to the expected naming convention of the C++ language standard. Would you like to adjust your selection for unique names?

DavidHammen commented 1 year ago

I rejoined GitHub just to comment on this. All identifiers that start with a double underscore are strictly reserved for use by the implementation (the compiler, the linker, etc.), as are all identifiers starting with a single underscore and immediately followed by an uppercase letter. In some places identifiers starting with a single underscore are strictly reserved as well. Violating this invokes undefined behavior, which means no diagnostic is required.

The easy solution to this is to never use any identifier that starts with a leading underscore. IIRC, that easy solution used to be in the JEOD coding standards.

Equally alarming is the using namespace std in jeod/models/environment/spice/verif/compare/include/io_utils.hh (the same file that contains _DEG2RAD). While I understand that this is just verif code, but using namespace std does not belong in any header file.

excaliburtb commented 1 year ago

thank you for the comments. this was legacy code from 2017 introduced by another team and hasn't been touched since then. As you said, it is for verification purposes, so it hasn't been audited or anything in a while. We created an issue to take care of it.

elfring commented 1 year ago

We created an issue to take care of it.

To which issue tracker do you refer here?

excaliburtb commented 1 year ago

We have an internal issue system that encompasses both the open-source and JSC internal content

zli0 commented 11 months ago

issue addressed in JEOD v5.1

elfring commented 11 months ago

:thought_balloon: I would find an other commit message more helpful for this issue than the information “merge trunk into master for JEOD v5.1 release”.

DavidHammen commented 8 months ago

@elfring All of JEOD used to restricted per the International Traffic in Arms Regulations (ITAR). The vast majority of JEOD has since had those ITAR restrictions removed, thereby enabling those unrestricted portions of JEOD to be released as NASA open source. Because a small portion of JEOD remains ITAR-restricted, JEOD is not maintained on GitHub (GitHub is not ITAR compliant). Instead, JEOD is maintained internally using ITAR-compliant (and also CMMI level 3 compliant) processes and tools. Upon making an internal JEOD release, a clone of the internal JEOD repo is made, but with the ITAR-restricted portions stripped out. This unrestricted clone then pushed to the open source version of JEOD as a single GitHub commit, which is why you see the rather useless commit message.