search

E3SM-Project / scorpio

A high-level Parallel I/O Library for structured grid applications
Other
21 stars 16 forks source link

Allocate compmap from heap instead of a static VLA #524

Closed bartgol closed 10 months ago

bartgol commented 1 year ago

This PR switches a static VLA array with a dynamic one, inside PIOc_init_decomp, to avoid a potential stack overflow.

This PR is needed in EAMxx in order to use pioc directly (bypassing F90 bridges).

bartgol commented 1 year ago

This PR is needed in EAMxx in order to use pioc directly (bypassing F90 bridges).

bartgol commented 10 months ago

@jayeshkrishna @dqwu I'm going through PRs/issues I have open, and found this one. Any thought of integrating it?

jayeshkrishna commented 10 months ago

We will have to recreate the branch off master and cherry pick the commit. @dqwu can you take a look at it?

dqwu commented 10 months ago

We will have to recreate the branch off master and cherry pick the commit. @dqwu can you take a look at it?

I think we can rebase this feature branch and add additional commits.

dqwu commented 10 months ago

@jayeshkrishna Please re-review the updated PR, thanks.

jayeshkrishna commented 10 months ago

Comment from @bartgol 

This PR switches a static VLA array with a dynamic one, inside
PIOc_init_decomp. While static VLA's are supported by compilers
and part of the C99 standard, it is probably best to use a dynamic
one. On one hand, they can still produce valgrind errors, on the
other hand they may cause stack overflow.

On top of that, static VLA can cause valgrind false positives, which
make debugging a bit harder.

This PR is needed in EAMxx in order to use pioc directly (bypassing
F90 bridges).
bartgol commented 10 months ago

Thanks!