Convert corehost hostmisc trace and fx_ver to C#128420
Open
Copilot wants to merge 11 commits into
Open
Conversation
Copilot
AI
changed the title
[WIP] Implement trace conversion in hostmisc files
Break out hostmisc C conversion slice (trace + fx_ver + PAL delegation)
May 20, 2026
Replaces trace.cpp with a pure-C trace.c implementation. The trace module is now C end-to-end on every platform: pal.windows.c and pal.unix.c are pure C calling Win32/POSIX directly, with no C++ in the trace call path. Key changes: * trace.cpp removed; trace.c provides trace_setup/enable/is_enabled/ verbose/info/warning/error/println/println_empty/flush plus the va_list variants (trace_*_v) and trace_set/get_error_writer. * trace.h keeps the trace::* C++ namespace as inline forwarders to the new trace_* C functions, so existing corehost C++ callers continue to work without source changes. * pal.h reorganized into a C-compatible section at the top (pal_char_t, _X with two-step expansion for C-mode MSVC, PAL_THREAD_LOCAL, APPHOST_PATH_MAX, DIR_SEPARATOR_STR, pal_str_* macros, extern \"C\" decls for pal_get_own_executable_path/pal_directory_exists/pal_getenv/ pal_xtoi) plus the existing pal:: namespace gated under #ifdef __cplusplus. * pal.unix.c provides the four pal_* functions trace.c needs on POSIX. pal.windows.c provides pal_get_own_executable_path (via GetModuleFileNameW) and pal_directory_exists (GetFileAttributesW + FILE_ATTRIBUTE_DIRECTORY check, matching Unix S_ISDIR semantics). pal_getenv and pal_xtoi are static inline in pal.h on Windows. * utils.h gates its C++ surface under #ifdef __cplusplus and adds an extern \"C\" declaration of utils_get_filename, used by trace.c when TRACEFILE points to a directory. utils.c implements just that one helper. * The original spin lock (std::atomic_flag) is replaced with minipal_mutex, lazy-initialized via InitOnceExecuteOnce on Windows and pthread_once on POSIX. The corehost root CMakeLists.txt now adds the minipal subdirectory; hostmisc/hostmisc_public link against minipal/minipal_objects (gated NOT CLR_CMAKE_TARGET_BROWSER), and libnethost/libhostfxr static archives bundle minipal_objects so the symbols ship in the public archives. * Trace behavior is preserved including the 'Tracing enabled @ <timestamp>' log line (a small strftime helper in trace.c emits the same UTC GMT format as the original pal::get_timestamp). * trace_enable now detects truncation when constructing the per-PID trace path and reports it as a file open error rather than silently using a bad path. trace_error_v emits a fixed fallback message via stderr/error_writer if format/allocation fails, matching the old C++ behavior where such failures propagated as exceptions. First split of #126367 (rest of the C-conversion work follows). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Moves the fx_ver semver parsing/comparison logic from the C++ class to a pure-C implementation in hostmisc/fx_ver.c. The C++ `fx_ver_t` class is preserved as a thin wrapper that delegates to the new `c_fx_ver_*` API, so existing C++ callers compile unchanged. Key changes: * New `hostmisc/fx_ver.h`: declares `c_fx_ver_t` + `c_fx_ver_init/ cleanup/set/is_empty/parse/compare/as_str` in an `extern \"C\"` block, with the existing `fx_ver_t` C++ class declarations gated under`#ifdef __cplusplus`. * New `hostmisc/fx_ver.c`: ports parse_internal, valid_identifier(s), try_stou, index_of_non_numeric, get_id_len, c_fx_ver_compare, and c_fx_ver_as_str using pal_char_t buffers + malloc/free for owned strings. The structure mirrors the original C++ logic. * Rewritten `fxr/fx_ver.cpp`: every member function (constructors, operators, as_str, compare, parse) now delegates to the C functions. `as_str`/`compare` borrow `pal::string_t::c_str()` pointers into a stack-allocated `c_fx_ver_t` (no allocation, no cleanup); `parse` copies the C-allocated strings out into `pal::string_t` and calls `c_fx_ver_cleanup`. Drops the validate-in-constructor `assert`s (they referenced the now-static C helpers). * `hostmisc/pal.h` adds three more C-callable string macros that fx_ver.c needs: `pal_strchr`, `pal_strncmp`, `pal_strtoul`. * CMake plumbing: `hostmisc/CMakeLists.txt` adds `fx_ver.c` and lists `fx_ver.h` in HEADERS; `hostcommon/files.cmake` and the test `test/fx_ver/CMakeLists.txt` are updated to point at the new `hostmisc/fx_ver.h` location. Validation: * Native host build clean on Windows Release (host subset). * Dedicated `test_fx_ver` semver test passes (exit code 0) against the new C-backed implementation. Second split of #126367. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Introduces a C-callable pal_getenv that returns a heap-allocated copy of the named environment variable's value (or NULL if unset/empty). The caller is responsible for free()ing the returned pointer. Matches the existing C++ pal::getenv semantics, which always allocates into a pal::string_t. Choosing the always-allocate API keeps the surface small (one parameter, one return) and lets call sites use a single unconditional free() for cleanup. Also replaces trace.c's get_host_env_var with a thin DOTNET_HOST_<name> / COREHOST_<name> fallback wrapper around pal_getenv. trace_setup and trace_enable now use the heap-allocated values directly and free() them at the end of the call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Four spots in trace.c had if/else pairs where both branches were single statements with no braces. Add braces consistently so every if/else in the new C code follows the 'if there is an else, both branches are braced' rule. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Moves the two remaining ifdef'd platform-specific calls in trace.c into pal.h as macros, alongside the existing pal_xtoi / pal_str* family: * pal_get_pid() -> ((int)GetCurrentProcessId()) / ((int)getpid()) * pal_file_open(p, m) -> _wfsopen(p, m, _SH_DENYNO) / fopen(p, m) trace.c no longer needs to ifdef around get-pid or fopen, and the Windows-only <share.h> / Unix-only <unistd.h> includes drop out of trace.c (share.h is now pulled in via pal.h's Windows section). No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Replaces the bodies of the C++ pal:: implementations with calls to the new C pal_* APIs: * pal::is_directory -> pal_directory_exists (both) * pal::getenv -> pal_getenv (both) * pal::get_own_executable_path -> pal_get_own_executable_path (both) Where the original C++ behavior went beyond what the previous C contract supported, the C side is extended so the C++ wrappers can stay trivial: * pal_get_own_executable_path is now an always-allocate API (returns pal_char_t* / NULL, caller frees) -- consistent with pal_getenv. On Windows it does the GetModuleFileNameW doubling-buffer loop (starting at MAX_PATH = 260) that the prior GetModuleFileNameWrapper had, so paths longer than the old APPHOST_PATH_MAX still work and short paths use a smaller initial allocation than the old 4 KB std::vector wrapper. On Unix it just returns the malloc'd result of minipal_getexepath. trace.c is updated for the new signature (free the result). * pal_getenv on Windows now emits trace_warning on non-ERROR_ENVVAR_NOT_FOUND GetEnvironmentVariableW failures, matching the warning the C++ pal::getenv used to emit. Putting the warning inside pal_getenv (rather than capturing GetLastError after it returns NULL in the C++ wrapper) avoids any fragility around malloc/free perturbing GetLastError. Pure-define wrappers (pal_strlen, pal_str_printf, pal_xtoi, pal_get_pid, pal_file_open, etc.) are intentionally not delegated -- they are already trivial macros and the C++ pal:: inlines compile to the same calls. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Copilot
AI
changed the title
Break out hostmisc C conversion slice (trace + fx_ver + PAL delegation)
Convert corehost hostmisc trace and fx_ver to C
May 21, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the corehost “hostmisc” C-migration by replacing the existing C++ trace implementation with a C implementation + C-callable header surface, and by moving the SemVer parser (fx_ver) into a C implementation with a thin C++ wrapper retained for existing callers. It also reshapes pal / utils so they can be consumed from C code, and updates CMake wiring to pull in minipal/minipal_objects where needed.
Changes:
- Replace
hostmisc/trace.cppwithhostmisc/trace.cand expose a Ctrace_*API intrace.h(with inline C++ shims for existingtrace::callers). - Move SemVer parsing into
hostmisc/fx_ver.{c,h}(C API) and reworkfxr/fx_ver.cppinto a C++ wrapper overc_fx_ver_*. - Add/adjust C PAL + utils helpers and update CMake to include/link
minipal/minipal_objectsfor host binaries/static libraries.
Show a summary per file
| File | Description |
|---|---|
| src/native/minipal/CMakeLists.txt | Disables CMAKE_INCLUDE_CURRENT_DIR in minipal directory scope. |
| src/native/corehost/CMakeLists.txt | Adds minipal subdirectory to non-browser corehost builds. |
| src/native/corehost/hostmisc/CMakeLists.txt | Switches hostmisc sources to C implementations and wires minipal/minipal_objects linkage (non-browser). |
| src/native/corehost/hostmisc/pal.h | Reorganizes into a C-compatible section plus C++ pal:: section under __cplusplus. |
| src/native/corehost/hostmisc/pal.windows.c | Adds Windows C implementations for pal_getenv, pal_get_own_executable_path, pal_directory_exists. |
| src/native/corehost/hostmisc/pal.unix.c | Adds Unix C implementations for pal_getenv, pal_get_own_executable_path, pal_directory_exists. |
| src/native/corehost/hostmisc/pal.windows.cpp | Redirects selected C++ pal:: functions to the new C implementations. |
| src/native/corehost/hostmisc/pal.unix.cpp | Redirects selected C++ pal:: functions to the new C implementations. |
| src/native/corehost/hostmisc/utils.h | Makes header C-safe (guards C++-only pieces) and declares utils_get_filename for C callers. |
| src/native/corehost/hostmisc/utils.c | Adds C implementation of utils_get_filename used by trace.c. |
| src/native/corehost/hostmisc/trace.h | Introduces a C trace_* API and C++ inline forwarders for trace::. |
| src/native/corehost/hostmisc/trace.c | New C implementation of trace functionality (locking, env var reading, output). |
| src/native/corehost/hostmisc/fx_ver.h | New C SemVer API (c_fx_ver_*) plus C++ fx_ver_t wrapper declaration. |
| src/native/corehost/hostmisc/fx_ver.c | New C SemVer parsing/comparison implementation. |
| src/native/corehost/fxr/fx_ver.cpp | Replaces prior C++ semver logic with a thin wrapper over c_fx_ver_*. |
| src/native/corehost/hostcommon/files.cmake | Updates include/header list to reference moved hostmisc/fx_ver.h. |
| src/native/corehost/nethost/CMakeLists.txt | Adds minipal_objects to static lib link line. |
| src/native/corehost/fxr/staticlib/CMakeLists.txt | Adds minipal_objects to static libhostfxr link line. |
| src/native/corehost/test/fx_ver/CMakeLists.txt | Updates include path for the relocated fx_ver.h. |
| src/native/corehost/hostmisc/trace.cpp | Removed (replaced by trace.c). |
| src/native/corehost/fxr/fx_ver.h | Removed (replaced by hostmisc/fx_ver.h). |
Copilot's findings
- Files reviewed: 21/21 changed files
- Comments generated: 2
Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
elinor-fung
reviewed
May 22, 2026
Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
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.
Split of #126367: converting the trace and fx_ver modules in
src/native/corehost/hostmiscfrom C++ to C, and reshapingpal/utilsfor a C-callable surface.These are pieces that both
apphostandnethostwould need (also,fxr_resolver, but didn't include that yet). My thought is to get the shared pieces in and then theapphostandnethostconversions with their separate sets of complications can be iterated on in parallel.cc @dotnet/appmodel @AaronRobinsonMSFT
trace.creplacestrace.cpptrace.hexposes a Ctrace_*API with C++ inline forwarders.hostmisc/fx_ver.{c,h}(moved fromfxr/) implements the semver parser in Cfxr/fx_ver.cppis now a thin C++ wrapper overc_fx_ver_t.pal.h/utils.hreorganized into a C-compatible section and a C++pal::section under#ifdef __cplusplus.pal.*.c/utils.cprovide implementations for functions needed for trace and fx_ver.pal.*.cpp/utils.cppversions of those functions are wrappers around the C implementations.minipallinked in for host binaries (minipal_objectsfor static libraries)Windows x64 Release
Linux x64 Release
Static libraries (
libhostfxr.a/libandlibnethost.a/lib) do grow due to includingminipal_objects- consumers can (probably should) link with dead code elimination.