-
Notifications
You must be signed in to change notification settings - Fork 26
Fix compiler warnings #51
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: master-old
Are you sure you want to change the base?
Conversation
| CFLAGS="$CFLAGS -Werror" | ||
| CPPFLAGS="$CPPFLAGS -Werror" | ||
| CXXFLAGS="$CXXFLAGS -Werror" |
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.
i'd preferably like to make this possible to disable (like through a --disable-werror flag), but i'm not sure how to exclude this flag from $ac_unrecognized_opts.
|
i see these lines a few times in what's |
| // FIXME: should be a readall. Check for return error code. | ||
| if (read(pipefd_out[0], &lh_info, sizeof lh_info) < sizeof lh_info) { | ||
| if ((unsigned long) read(pipefd_out[0], &lh_info, sizeof lh_info) | ||
| < sizeof lh_info) { |
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.
read can return -1, make it unsigned long will be 2^32 - 1 and passed this condition check, which is not correct.
| MPI_Request request; | ||
| MPI_Comm realComm = VIRTUAL_TO_REAL_COMM(comm); | ||
| int flag = 0; | ||
| // Consider removing the #if 0 segment below? |
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.
I don't know what's the purpose of the #if 0 bloc below, may keep it for debugging purposes.
But in C++, we usually define the variable right before we use it, so you can move this line to 145 if we don't want to remove that #if 0 block.
| MTCP_ASSERT(getMappedArea(&stack_area, "[stack]") == 1); | ||
| end1 = min(stack_area.endAddr - 4 * GB, highMemStart - 4 * GB); | ||
| end1 = (char *) min((uint64_t) stack_area.endAddr - 4 * GB, | ||
| (uint64_t) highMemStart - 4 * GB); |
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.
We should be very careful when performing minus on uint64_t, making sure the first is greater than the second.
| // If we want to test this, we can add code to do a trial mremap with a page | ||
| // before vvar and after vdso, and verify that we get an EFAULT. | ||
| // In May, 2020, on Cori and elsewhere, vvar is 3 pages and vdso is 2 pages. | ||
| char *vdsoStart = (char *)mygetauxval(environ, AT_SYSINFO_EHDR); |
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.
What is this for and why removing this line?
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.
this is a currently unused variable that was previously used when determining where to temporarily map the vdso/vvar segments to. there used to be a function mysetauxval that set AT_SYSINFO_EHDR so this line could read for it, but since that function is no longer in use we can remove this one as well.
This pull request fixes some compiler warnings generated when building MANA on CentOS - some were caused by some of my older PRs.