Fix compiler warnings #51
Conversation
| CFLAGS="$CFLAGS -Werror" | ||
| CPPFLAGS="$CPPFLAGS -Werror" | ||
| CXXFLAGS="$CXXFLAGS -Werror" |
There was a problem hiding this comment.
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 |
| @@ -333,7 +333,8 @@ startProxy() | |||
| // Read from stdout of lh_proxy full lh_info struct, including orig memRange. | |||
| close(pipefd_out[1]); // close write end of pipe | |||
| // 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.
read can return -1, make it unsigned long will be 2^32 - 1 and passed this condition check, which is not correct.
| @@ -130,7 +130,8 @@ TwoPhaseAlgo::commit_begin(MPI_Comm comm) | |||
| // Set state again incase we returned from beforeTrivialBarrier | |||
| 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.
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.
| Area stack_area; | ||
| 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.
We should be very careful when performing minus on uint64_t, making sure the first is greater than the second.
| @@ -710,8 +714,6 @@ main(int argc, char *argv[], char **environ) | |||
| // 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.
What is this for and why removing this line?
There was a problem hiding this comment.
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.