-
Notifications
You must be signed in to change notification settings - Fork 393
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
Fix #8312 - EnergyPlus Build Warnings on gcc >= 9 #8391
Conversation
… num, size_t size);` num = Number of elements to allocate. size = Size of each element https://www.cplusplus.com/reference/cstdlib/calloc/ Reorder arguments to calloc to match signature: `void* calloc (size_t num, size_t size);` num = Number of elements to allocate. size = Size of each element https://www.cplusplus.com/reference/cstdlib/calloc/
…le the warnings in question when satisfied. c
…Werror=array-bounds]
if (CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 9.0) | ||
target_link_libraries( energypluslib util ) | ||
endif() |
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.
Wouldn't link without this on GCC 10. Undefined reference to forkpty
and openpty
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.
@@ -104,6 +104,7 @@ ELSEIF ( CMAKE_COMPILER_IS_GNUCXX OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" | |||
ADD_CXX_DEFINITIONS("-pipe") # Faster compiler processing | |||
ADD_CXX_DEFINITIONS("-pedantic") # Turn on warnings about constructs/situations that may be non-portable or outside of the standard | |||
ADD_CXX_DEFINITIONS("-Wall -Wextra") # Turn on warnings | |||
ADD_CXX_DEFINITIONS("-Werror") # Treat warnings as errors |
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'm curious to see if clang will throw or not. We can remove this line if it's problematic, but it worked on GCC and we might as well turn it on to avoid having warnings sneak into the codebase.
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.
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.
(Enabling /W4 or /Wall on windows would be interesting too)
EnergyPlus/cmake/CompilerFlags.cmake
Line 28 in f586325
STRING (REGEX REPLACE "/W3" "/W1" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") # Increase to /W2 then /W3 as more serious warnings are addressed (using regex to avoid VC override warnings) |
fmiInteger *sizefmuResFolder, | ||
fmiReal *timeOut, | ||
fmiInteger *visible, | ||
fmiInteger *interactive, | ||
fmiInteger *loggingOn, | ||
fmiInteger *index); |
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.
Plenty of changes are due to the fact that I clang-formatted all the files in the FMI and FMUParser folders, as they were pretty messy.
/// that mapped to fmiFunctions and will be exported | ||
/// in .dll | ||
/////////////////////////////////////////////////////// | ||
|
||
#include "FMI/fmiModelFunctions.h" |
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.
Include problem.
|
||
// load lib by specifying path to the binaries | ||
if (loadLib(trimfmuWorkingFolder, fmuInstances[_c->index]->modelID, fmuInstances[_c->index])) { | ||
memcpy(fmiVersionNumber, err_msg, strlen(err_msg)); |
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.
stringop-overflow
warning. (side note: fmiVersionNumber
is supposed to be allocated outside of the function, hoping that it is indeed, and correctly...)
|
||
if (fmu.modelDescription==NULL){ | ||
printfError("Fail to parse xml file \"%s\".\n", xmlPat); | ||
free(xmlPat); |
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.
Free xmlPat, and as soon as it's not used anymore
tmp = malloc(length*sizeof(char)); | ||
strcpy(tmp, fmuFilNam); | ||
length = strlen(fmuFilNam); | ||
tmp = (char *)malloc((length+1) * sizeof(char)); |
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.
malloc to length + 1
free(tmp); | ||
free(filNam); | ||
free(ext); |
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.
free eveything
|
||
objNam = (char *)calloc((length + 1), sizeof(char)); | ||
// Copy the fmufilNam without extension as objNam | ||
if (memcpy(objNam, fmuFilNam, length) == NULL) { |
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.
stringop-overflow
warning.
return -1; | ||
} | ||
|
||
// This is pretty useless as calloc will initialize to 0 (which is in effect the null-terminator) |
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.
meh.
Confirmed that this fixes build warnings with gcc >= 9. Pulling in develop one more time, then this should go right in. |
This looks good. Merging. |
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.