Use native filesystem paths across API boundaries#8231
Draft
plafosse wants to merge 1 commit into
Draft
Conversation
Carry paths through the C ABI as BNPath objects containing native platform code units instead of forcing UTF-8 or active-code-page strings. Update the C++, Python, and Rust APIs to preserve filesystem path types until explicit display or interchange boundaries. Centralize printable path formatting, add a C++ IsDatabase path wrapper, and reuse shared path conversion helpers. Add path round-trip coverage for Python FFI and Rust/C++ path-facing APIs, including Windows wide paths and POSIX byte paths.
plafosse
force-pushed
the
test_native_path_roundtrip
branch
from
9332000 to
f359cae
Compare
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
1 participant
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.
Summary
This is the API-side portion of a major refactor of how filesystem paths are handled throughout Binary Ninja. Before this change, several path flows round-tripped paths through UTF-8 or narrow strings, which was especially lossy on Windows. This changeset keeps filesystem paths as path objects until an explicit boundary requires text or byte-oriented data.
Within core, filesystem paths should not be handled as
std::stringexcept at explicit boundaries:PrintablePath.PathToUtf8String.Although this touches a lot of code, most changes are mechanical replacements of string path plumbing with native path types, or replacing C-style path handling with idiomatic C++. The main new API-side code is in
pathhelpers.h, the public API wrappers, the binding generator, and the new tests.API Changes
char*,str, or equivalent string types:std::filesystem::pathBNPathpathlib.Pathstd::path::PathBNPathis now the opaque C ABI representation for filesystem paths and wrapsstd::filesystem::pathon the core side.BNAllocPath,BNFreePath, and plural forms.BNAllocPath/BNFreePath.BNAllocPath/BNFreePathto simplify C++ API wrappers.BN_OWNEDBN_BORROWEDBN_NULLABLEBN_FREED_BYBN_OUT_COUNTBN_PATH_ELEMENT_COUNTReview Notes
I have not done as much focused review of the Python and Rust API surfaces as the core/C++ changes. Please pay particular attention to those areas, especially Rust path ownership and conversion behavior. The main goal of this draft PR is to get the full changeset into CI and make it easier to discuss and review.
Validation
git -C api diff --check origin/devcmake --build cmake-build-relwithdebinfo --target binaryninjaapi binaryninjaui cpp_unit -j 8cmake --build cmake-build-relwithdebinfo --target sharedcachecore -j 8cmake --build cmake-build-relwithdebinfo --target debuggercore debuggerui -j 8Companion core PR: https://github.com/Vector35/binaryninja/pull/1554