Fix #11244 - Allow building with AppleClang 17#11246
Conversation
| size_t n_BCVTB_HOME = strlen(BCVTB_HOME); | ||
| size_t n_fileName = strlen(fileName); | ||
| size_t n_xmlPath = strlen(xmlPath); // 9 | ||
| size_t n_jarPath = strlen(jarPath); // 35 | ||
| size_t n_dtdName = strlen(dtdName); // 13 | ||
|
|
||
| // Verify that variables.dtd exists and is readable | ||
| { | ||
| // Add 1 for the null terminator | ||
| size_t n_needed = n_BCVTB_HOME + n_xmlPath + n_dtdName + 1; | ||
|
|
||
| char* dtdFileName = (char*)malloc(sizeof(char) * n_needed); | ||
| if (!dtdFileName) { | ||
| fprintf(stderr, | ||
| "Error: Memory allocation for 'dtdFileName' failed in check_variable_cfg_Validate when parsing file '%s'. \n" | ||
| " Program aborting.\n", | ||
| fileName); | ||
| // free(dtdFileName); | ||
| return -1; | ||
| } | ||
|
|
||
| snprintf(dtdFileName, n_needed, "%s%s%s", BCVTB_HOME, xmlPath, dtdName); | ||
| FILE* dtdF = fopen(dtdFileName, "r"); | ||
| if (!dtdF) { | ||
| fprintf(stderr, "Error: Cannot open '%s'.\n", dtdFileName); | ||
| free(dtdFileName); | ||
| return -1; | ||
| } else { | ||
| fclose(dtdF); | ||
| } |
There was a problem hiding this comment.
Calculate the size needed properly, use snprintf not sprintf.
A test bed that will showcase the before/after is provided for convenience here: https://compiler-explorer.com/z/xoW58hn3j
There was a problem hiding this comment.
I'm also freeing as soon as I can, and scoping variables better.
third_party/BCVTB/utilXml.c
Outdated
| dtdFileName = (char*)malloc(sizeof(char) * (strlen(BCVTB_HOME) + 30)); | ||
| if (NULL == command) { |
There was a problem hiding this comment.
Note that this check on line 879 was wrong, it was checking the wrong variable...
Doesn't matter that much because malloc will never fail on the systems we run this on, but still
| // Add 1 for the null terminator | ||
| size_t n_needed = 18 + 2 * n_BCVTB_HOME + n_jarPath + n_xmlPath + n_fileName + 1; | ||
| // int n_needed = snprintf(NULL, 0, cmd_fmt, BCVTB_HOME, jarPath, fileName, BCVTB_HOME, xmlPath) + 1; |
There was a problem hiding this comment.
I have considered calling snprintf with a null ptr and bufsize of zero to let it calculate the needed size for me, but this I have everything I need I decided not to.
| if(NOT MSVC) | ||
| target_compile_options(glfw PRIVATE -Wno-deprecated-declarations) | ||
| endif() |
There was a problem hiding this comment.
For penumbra's glfw, just ignore deprecated declarations for now.
| "Error: Memory allocation for 'dtdFileName' failed in check_variable_cfg_Validate when parsing file '%s'. \n" | ||
| " Program aborting.\n", | ||
| fileName); | ||
| // free(dtdFileName); |
There was a problem hiding this comment.
I'm omitting this because it's pointless.
(Note that I could have left it, because free on a null pointer has no effect)
Pull request overview
Description of the purpose of this PR
-Wno-deprecated-declarations) for penumbra's glfwReview without the first commit, which is just clang format
https://github.com/NREL/EnergyPlus/pull/11246/files/8d0e1c0208bae1d0d40159c4a62136ceaf833b30..3405fb1bf5546c80646a996282eefa9391720421
Pull Request Author
Reviewer