-
Notifications
You must be signed in to change notification settings - Fork 24
lower_half/mmap64.c: mmaps[]: add sentinel #351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
82eef31
to
8e53f1d
Compare
mpi-proxy-split/lower-half/mmap64.c
Outdated
memset(&mmaps[i], 0, sizeof(mmaps[i])); | ||
mmaps[i].addr = MAP_FAILED; // Mark those mmaps past numMmapRegions, for GDB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memset should take care of setting all the variables to 0x0. We do not need to set MAP_FAILED separately. Also, the comment does not reflect the code. The loop is only till numMmapRegions, not past that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to add MAP_FAILED after the for loop? The value of i
would then be numMmapRegions
.
8e53f1d
to
eb55bed
Compare
mpi-proxy-split/lower-half/mmap64.c
Outdated
memset(&mmaps[i], 0, sizeof(mmaps[i])); | ||
mmaps[i].addr = MAP_FAILED; // Mark those mmaps past numMmapRegions, for 8000 GDB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to add MAP_FAILED after the for loop? The value of i
would then be numMmapRegions
.
eb55bed
to
69eedbb
Compare
69eedbb
to
2a5cfbf
Compare
@JainTwinkle, Thanks for the comment about:
Good catch! I've now changed it to set the 'addr' to MAP_FAILED for all mmaps elements. Since 'numMmapRegions' is 0 at the end, I could set it just for the zero-th element. But it's safer to set it for all 'mmaps' elements. |
This adds a sentinel (MAP_FAILED) to the mmaps[] array, and then checks for it in the mpi_plugin.cpp:DMTCP_EVENT_PRECHECKPOINT.
It also renames numRegions to numMmapRegions in lower_half, and improves the documentation in mmap64.c where we reserve a large mmap'ed region for mmap64.c,, starting at 0x2aab00000000;.