Revert "Merge pull request #7134 from BOINC/vko_fix_compile_warnings"#7137
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts the earlier “compile warning cleanup” changes, restoring legacy C/C++ APIs and signatures across BOINC core libraries, scheduler code, Windows-specific utilities, and unit tests in order to recover prior behavior and fix regressions (notably on Windows).
Changes:
- Reverted multiple API/signature changes (e.g., removal of extra buffer-length parameters) and updated call sites accordingly.
- Restored older Windows/CRT behaviors (e.g., stdio calls, Win32 version/platform logic) and adjusted build flags/macros.
- Updated unit tests to match the restored interfaces and legacy string-handling behavior.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit-tests/win/pch.h | Removes HAVE_STRNCPY_S / HAVE_STRNCAT_S defines from Windows unit test PCH. |
| tests/unit-tests/lib/test_util.cpp | Renames the unit test namespace to match legacy naming. |
| tests/unit-tests/lib/test_url.cpp | Removes str_replace include; switches test namespace and string copy usage. |
| tests/unit-tests/lib/test_str_util.cpp | Updates tests to match restored str_util APIs and adds additional coverage. |
| tests/unit-tests/lib/test_shmem.cpp | Renames the unit test namespace to match legacy naming. |
| tests/unit-tests/lib/test_parse.cpp | Renames the unit test namespace and adjusts string copy usage. |
| tests/unit-tests/lib/test_md5_file.cpp | Renames the unit test namespace to match legacy naming. |
| tests/unit-tests/lib/test_base64.cpp | Renames the unit test namespace to match legacy naming. |
| sched/sched_types.cpp | Updates str_replace() calls to match reverted signature. |
| sched/sched_send.cpp | Updates secs_to_hmsf() calls to match reverted signature. |
| sched/make_work.cpp | Restores older header metadata and reverts replace_file_name() signature/calls. |
| lib/win_util.cpp | Restores older Windows utility behaviors (registry string init, error formatting). |
| lib/util.cpp | Adjusts platform macros/includes and restores legacy run_command() behavior around FCGI. |
| lib/str_util.h | Restores older function signatures and APIs (e.g., secs_to_hmsf, adds legacy declarations). |
| lib/str_util.cpp | Restores legacy implementations (including time formatting and string operations). |
| lib/str_replace.h | Removes non-Windows “secure CRT” compatibility shims that were added previously. |
| lib/shmem.cpp | Restores legacy Windows shared-memory handling (including Win9x path). |
| lib/project_init.cpp | Reverts file I/O wrappers back to stdio functions. |
| lib/procinfo_win.cpp | Restores legacy Win9x/NT platform behavior and older memory/process info handling. |
| lib/proc_control.cpp | Adjusts thread suspend/resume return-value logic. |
| lib/prefs.cpp | Restores legacy localtime behavior and adjusts file-open logic for FCGI builds. |
| lib/parse.h | Restores older signatures for XML/string utilities and adds legacy helper declarations. |
| lib/parse.cpp | Restores legacy XML/string utility implementations (including string replacement/escaping). |
| lib/opencl_boinc.cpp | Restores legacy _USING_FCGI_ behavior for XML output. |
| lib/mfile.h | Reverts MFILE buffer length type and method signatures to legacy versions. |
| lib/mfile.cpp | Reverts MFILE implementation details and string-copy behavior. |
| lib/mem_usage.cpp | Restores older Windows-specific memory usage logic. |
| lib/Makefile.mingw | Reverts MinGW flags to the prior set. |
| lib/gui_rpc_client.cpp | Updates buffer-length handling to match reverted MFILE::get_buf() signature. |
| lib/filesys.h | Restores older signatures for path utilities (relative_to_absolute, boinc_path_to_dir). |
| lib/filesys.cpp | Restores legacy implementations for path utilities and Windows file truncation. |
| lib/diagnostics.cpp | Restores legacy diagnostics behaviors and adjusts file I/O calls. |
| lib/diagnostics_win.cpp | Restores legacy Win9x/NT registry behavior. |
| lib/daemonmgt_win.cpp | Restores legacy Windows service-control invocation behavior. |
| lib/coproc.h | Restores legacy string handling and removes forward-decl workarounds. |
| lib/coproc.cpp | Restores legacy tokenization and JSON formatting behaviors. |
| lib/cert_sig.cpp | Restores legacy file I/O paths with FCGI conditional logic. |
| lib/cc_config.cpp | Adjusts includes to match prior state. |
| lib/boinc_win.h | Restores legacy CRT mappings and disables MSVC deprecation warnings. |
| lib/boinc_stdio.h | Removes MSVC-specific fopen_s/freopen_s wrappers and restores legacy behavior. |
| client/project.cpp | Updates relative_to_absolute() usage to match reverted signature. |
| client/gui_rpc_server_ops.cpp | Reverts RPC output-length handling to match MFILE changes and legacy formatting. |
| client/client_types.cpp | Updates relative_to_absolute() usage to match reverted signature. |
| client/client_state.cpp | Updates relative_to_absolute() usage to match reverted signature. |
| client/async_file.cpp | Updates boinc_path_to_dir() usage to match reverted signature. |
| client/app.cpp | Updates relative_to_absolute() usage to match reverted signature. |
| client/app_start.cpp | Updates relative_to_absolute() usage to match reverted signature. |
| api/boinc_api.cpp | Updates process-id formatting and relative_to_absolute() usage to match reverted signature. |
Comments suppressed due to low confidence (3)
lib/parse.cpp:324
- extract_venue(): switching from bounded concatenation to plain strncat() can overflow out because strncat() doesn’t know the destination buffer size. Since len is available here, keep concatenation bounded to remaining space.
q = in;
strcpy(out, "");
while (1) {
p = strstr(q, "<venue");
if (!p) {
strlcat(out, q, len);
break;
}
strncat(out, q, p-q);
q = strstr(p, "</venue>");
if (!q) break;
q += venue_close_tag_len;
}
lib/parse.cpp:360
- non_ascii_escape() now uses strcpy() into out without respecting len. The subsequent "p > out + len - 8" check happens after the copy, so it can already be too late. Use strlcpy (or equivalent) with the remaining buffer size.
void non_ascii_escape(const char* in, char* out, int len) {
char buf[256], *p;
p = out;
for (; *in; in++) {
int x = (int) *in;
x &= 0xff; // just in case
if (x>127) {
snprintf(buf, sizeof(buf), "&#%d;", x);
strcpy(p, buf);
p += strlen(buf);
} else {
*p++ = (char)x;
}
if (p > out + len - 8) break;
}
*p = 0;
lib/coproc.cpp:180
- summary_json() uses sprintf() to write JSON into a caller-provided buffer without any size parameter. If model/vendor strings are long, this can overflow buf2. Prefer snprintf with an explicit buffer length (e.g., pass sizeof(buf2) into summary_json).
void summary_json(
char *buf,
const char *type, const char *model, int count, int ram_mb,
const char *driver_version, int opencl_version
) {
sprintf(buf,
" {\n"\
" \"type\": \"%s\",\n"\
" \"model\": \"%s\",\n"\
" \"count\": %d,\n"\
" \"ram_mb\": %d,\n"\
" \"driver_version\": \"%s\",\n"\
" \"opencl_version\": \"%d\"\n"\
" }",
type, model, count, ram_mb, driver_version, opencl_version
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
181
to
184
| if (dwRet != 0) { | ||
| // include the hex error code as well | ||
| snprintf(pszBuf, iSize, "%s (0x%lx)", lpszTemp, dwError); | ||
| snprintf(pszBuf, iSize, "%s (0x%x)", lpszTemp, dwError); | ||
| if (lpszTemp) { |
Comment on lines
81
to
83
| lpszValue = (LPSTR) malloc(dwSize); | ||
| (*lpszValue) = '\0'; | ||
| (*lpszValue) = NULL; | ||
|
|
Comment on lines
100
to
103
| // Make the buffer big enough for the expanded string | ||
| lpszExpandedValue = (LPSTR) malloc(lReturnValue); | ||
| (*lpszExpandedValue) = '\0'; | ||
| (*lpszExpandedValue) = NULL; | ||
| dwSize = lReturnValue; |
Comment on lines
+939
to
945
| void relative_to_absolute(const char* relname, char* path) { | ||
| boinc_getcwd(path); | ||
| if (strlen(relname)) { | ||
| strlcat(path, "/", path_len); | ||
| strlcat(path, relname, path_len); | ||
| strcat(path, "/"); | ||
| strcat(path, relname); | ||
| } | ||
| } |
Comment on lines
+998
to
1005
| void boinc_path_to_dir(const char* path, char* dir) { | ||
| strcpy(dir, path); | ||
| char* p = strrchr(dir, '/'); | ||
| if (p) { | ||
| *p = 0; | ||
| } else { | ||
| strlcpy(dir, ".", dir_len); | ||
| strcpy(dir, "."); | ||
| } |
Comment on lines
+168
to
+180
| if (smallest_timescale==0) { | ||
| snprintf( sec_buf, sizeof(sec_buf), "%.2f sec ", seconds ); | ||
| } else if (seconds > 1 && smallest_timescale < 0) { | ||
| snprintf( sec_buf, sizeof(sec_buf), "%d sec ", (int)seconds ); | ||
| } else { | ||
| safe_strcpy( sec_buf, "" ); | ||
| } | ||
| // the "-0.05" below is to prevent it from printing 60.0 sec | ||
| // when the real value is e.g. 59.91 | ||
| // | ||
| sprintf(buf, "%s%s%s%s%s", year_buf, day_buf, hour_buf, min_buf, sec_buf); | ||
|
|
||
| return 0; |
Comment on lines
373
to
385
| for (; *in; in++) { | ||
| int x = (int) *in; | ||
| x &= 0xff; // just in case | ||
| if (x == '<') { | ||
| strlcpy(p, "<", len-(p-out)); | ||
| strcpy(p, "<"); | ||
| p += 4; | ||
| } else if (x == '&') { | ||
| strlcpy(p, "&", len-(p-out)); | ||
| strcpy(p, "&"); | ||
| p += 5; | ||
| } else if (x>127) { | ||
| snprintf(buf, sizeof(buf), "&#%d;", x); | ||
| strlcpy(p, buf, len-(p-out)); | ||
| strcpy(p, buf); | ||
| p += strlen(buf); |
Comment on lines
33
to
36
| class MFILE { | ||
| char* buf; // NULL-terminated | ||
| size_t len; | ||
| int len; | ||
| FILE* f; |
Comment on lines
105
to
116
| size_t MFILE::write(const void *ptr, size_t size, size_t nitems) { | ||
| buf = (char *)realloc_aux( buf, len+(size*nitems)+1 ); | ||
| if (!buf) { | ||
| boinc::fprintf(stderr, | ||
| "ERROR: realloc() failed in MFILE::write(); len %zu size %zu nitems %zu\n", | ||
| "ERROR: realloc() failed in MFILE::write(); len %d size %zu nitems %zu\n", | ||
| len, size, nitems | ||
| ); | ||
| exit(1); | ||
| } | ||
| memcpy( buf+len, ptr, size*nitems ); | ||
| len += size*nitems; | ||
| len += (int)size*(int)nitems; | ||
| buf[len] = 0; |
Comment on lines
+174
to
+182
| int strcatdup(char*& p, char* buf) { | ||
| char* new_p = (char*)realloc(p, strlen(p) + strlen(buf)+1); | ||
| if (!new_p) { | ||
| return ERR_MALLOC; | ||
| } | ||
| p = new_p; | ||
| strcat(p, buf); | ||
| return 0; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit d2b3ff7, reversing changes made to 32a8674.
Summary by cubic
Reverts the compile‑warning cleanup to restore legacy APIs and behavior across core, lib, scheduler, and tests. Fixes Windows build/runtime regressions and aligns unit tests with the pre-change interfaces.
Written for commit 667484e. Summary will update on new commits.