Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix portability issue in parse_macros function used by the MPAS registry #1229

Merged

Conversation

mgduda
Copy link
Contributor

@mgduda mgduda commented Sep 9, 2024

This PR fixes a portability issue in the parse_macros function used by the MPAS registry.

The parse_macros function that is used by the MPAS registry parse tool previously made use of the state/saveptr/lasts argument to strtok_r when parsing command-line macro definitions. This was not portable and resulted in incorrectly generated output on some systems.

This PR reworks the parse_macros function so that it no longer uses the state/saveptr/lasts argument to strtok_r and instead relies only on the return value of strtok_r to provide portability across systems with different strtok_r implementations.

@mgduda mgduda changed the base branch from develop to hotfix-v8.2.2 September 9, 2024 21:33
The parse_macros function that is used by the MPAS registry 'parse' tool
previously made use of the state/saveptr/lasts argument to strtok_r when
parsing command-line macro definitions. This was not portable and resulted
in incorrectly generated output on some systems.

This commit reworks the parse_macros function so that it no longer uses the
state/saveptr/lasts argument to strtok_r and instead relies only on the return
value of strtok_r to provide portability across systems with different strtok_r
implementations.
@mgduda mgduda force-pushed the framework/strtok_portability_fix branch from 0331cfd to b74756f Compare September 9, 2024 21:44
@mgduda mgduda marked this pull request as ready for review September 9, 2024 21:44
@mgduda mgduda requested a review from gdicker1 September 9, 2024 21:45
Copy link
Collaborator

@gdicker1 gdicker1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

@mgduda mgduda merged commit 0a5edb2 into MPAS-Dev:hotfix-v8.2.2 Sep 13, 2024
mgduda added a commit that referenced this pull request Sep 20, 2024
This merge addresses several issues in the MPAS-Atmosphere model and in the MPAS
infrastructure. Specific changes in this merge include:

* Fix to a portability issue in the MPAS registry 'parse' tool, which caused
  files in the src/core_<CORE>/inc directory to not be generated correctly at
  build time on some systems (PR #1229).

* Addition of two fields, brtemp and cldmask, that are needed by MPAS-JEDI.
  Although not needed by stand-alone MPAS-Atmosphere, these fields are
  associated with the 'jedi_da' package and therefore have no effect when
  MPAS-Atmosphere is run without setting config_jedi_da = true (PR #1232).

* Removal of a redundant query of the nCellsSolve dimension in the
  physics_run_init routine. The extra query had no impact on results, and its
  removal can be considered clean-up (PR #1236).
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants