Skip to content

fix(plugin): Use UTF-8 paths on Windows#2841

Open
geraldcombs wants to merge 2 commits intofalcosecurity:masterfrom
geraldcombs:windows-plugin-path
Open

fix(plugin): Use UTF-8 paths on Windows#2841
geraldcombs wants to merge 2 commits intofalcosecurity:masterfrom
geraldcombs:windows-plugin-path

Conversation

@geraldcombs
Copy link
Copy Markdown
Contributor

@geraldcombs geraldcombs commented Feb 13, 2026

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind test

/kind feature

/kind sync

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-modern-bpf

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

This ensures that we can load plugins on Windows independent of the system code page. It adds a requirement that plugin paths on Windows be encoded as UTF-8.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 13, 2026 •
edited
Loading

Please double check driver/SCHEMA_VERSION file. See versioning.

/hold

@poiana poiana requested a review from hbrueckner February 13, 2026 18:09
@poiana poiana requested a review from terror96 February 13, 2026 18:09
@geraldcombs
Copy link
Copy Markdown
Contributor Author

geraldcombs commented Feb 13, 2026

Tested locally by placing a plugin in a directory named "fptest-😀" and loading a capture using sinsp-example.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 13, 2026 •
edited
Loading

Perf diff from master - unit tests

    13.93%     -0.60%  [.] std::__shared_count<(__gnu_cxx::_Lock_policy)2>::_M_get_use_count() const
     6.23%     +0.57%  [.] std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count(std::__weak_count<(__gnu_cxx::_Lock_policy)2> const&, std::nothrow_t)
     9.39%     +0.48%  [.] sinsp_threadinfo::update_main_fdtable()
     3.84%     -0.44%  [.] sinsp_thread_manager::create_thread_dependencies(std::shared_ptr<sinsp_threadinfo> const&)
    15.54%     -0.42%  [.] std::__shared_ptr<sinsp_threadinfo, (__gnu_cxx::_Lock_policy)2>::__shared_ptr(std::__weak_ptr<sinsp_threadinfo, (__gnu_cxx::_Lock_policy)2> const&, std::nothrow_t)
     7.76%     -0.41%  [.] sinsp_threadinfo::get_fd_table()
     9.62%     -0.33%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release()
    11.41%     -0.33%  [.] sinsp_threadinfo::get_main_thread()
     4.18%     +0.32%  [.] thread_group_info::get_first_thread() const
     9.87%     +0.28%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_add_ref_lock_nothrow()

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            -0.0039         -0.0038           246           245           246           245
BM_sinsp_split_median                                          -0.0036         -0.0035           246           245           245           245
BM_sinsp_split_stddev                                          -0.3925         -0.3890             5             3             5             3
BM_sinsp_split_cv                                              -0.3901         -0.3866             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  +0.0817         +0.0817            69            74            69            74
BM_sinsp_concatenate_paths_relative_path_median                +0.0983         +0.0984            68            74            68            74
BM_sinsp_concatenate_paths_relative_path_stddev                -0.9553         -0.9608             1             0             1             0
BM_sinsp_concatenate_paths_relative_path_cv                    -0.9587         -0.9638             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0013         -0.0014            42            42            42            42
BM_sinsp_concatenate_paths_empty_path_median                   -0.0007         -0.0006            42            42            42            42
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.8809         -0.8879             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.8807         -0.8878             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  +0.0532         +0.0530            69            73            69            73
BM_sinsp_concatenate_paths_absolute_path_median                +0.0577         +0.0575            69            73            69            73
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.5965         -0.5927             1             0             1             1
BM_sinsp_concatenate_paths_absolute_path_cv                    -0.6168         -0.6132             0             0             0             0

terror96
terror96 reviewed Feb 16, 2026
View reviewed changes
terror96
terror96 reviewed Feb 16, 2026
View reviewed changes
terror96
terror96 reviewed Feb 16, 2026
View reviewed changes
terror96
terror96 reviewed Feb 16, 2026
View reviewed changes
terror96
terror96 reviewed Feb 16, 2026
View reviewed changes
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 18, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.38%. Comparing base (30a8910) to head (56cd265).

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #2841   +/-   ##
=======================================
  Coverage   75.38%   75.38%           
=======================================
  Files         295      295           
  Lines       31306    31306           
  Branches     4898     4898           
=======================================
  Hits        23601    23601           
  Misses       7705     7705
Flag Coverage Δ
libsinsp 75.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@terror96
Copy link
Copy Markdown
Contributor

terror96 commented Feb 19, 2026

ping @geraldcombs some formatting changes still required. ping @ekoops, I'm not sure what is going wrong with a few driver tests: 404?

@ekoops
Copy link
Copy Markdown
Contributor

ekoops commented Feb 19, 2026

ping @geraldcombs some formatting changes still required. ping @ekoops, I'm not sure what is going wrong with a few driver tests: 404?

I fixed them on the mainline. If @geraldcombs rebases, they will go away.

@ekoops ekoops added this to the 0.24.0 milestone Feb 19, 2026
@geraldcombs
Copy link
Copy Markdown
Contributor Author

geraldcombs commented Feb 19, 2026

I rebased and pushed from an environment that has clang-format installed.

terror96
terror96 previously approved these changes Feb 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@terror96 terror96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@poiana
Copy link
Copy Markdown
Contributor

poiana commented Feb 20, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geraldcombs, terror96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana added the lgtm label Feb 20, 2026
ekoops
ekoops requested changes Feb 20, 2026
View reviewed changes
@github-project-automation github-project-automation bot moved this from Todo to In progress in Falco Roadmap Feb 20, 2026
@poiana poiana removed the lgtm label Feb 20, 2026
@poiana poiana requested a review from terror96 February 20, 2026 09:17
@ekoops
Copy link
Copy Markdown
Contributor

ekoops commented Mar 3, 2026

Hey @geraldcombs , any update on this?

@geraldcombs geraldcombs force-pushed the windows-plugin-path branch from e3e115b to 62669ee Compare March 4, 2026 18:32
ekoops
ekoops requested changes Mar 5, 2026
View reviewed changes
userspace/libsinsp/examples/test.cpp Outdated
char** utf8_argv = nullptr;

if(utf16_argv && utf16_argc == argc) {
utf8_argv = (char**)malloc((argc + 1) * sizeof(char*));
Copy link
Copy Markdown
Contributor

@ekoops ekoops Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this can fail. We should add a check on the returned value here.

userspace/libsinsp/examples/test.cpp Outdated

#ifdef _WIN32
if(utf8_argv) {
free(utf8_argv);
Copy link
Copy Markdown
Contributor

@ekoops ekoops Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and here... At this point, it's better to have a separate function that check that utf8_argv is not NULL and free each of its elements

Copy link
Copy Markdown
Contributor

@terror96 terror96 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to this: I did not find a way to cleanly comment lines that were not part of this PR when I noticed a few missing free operations: #2872.

@terror96
Copy link
Copy Markdown
Contributor

terror96 commented Mar 6, 2026

Why is test-drivers-arm64 failing?

@geraldcombs
chore(driver): Suppress a Visual C++ warning
ff19858 
Suppress

    C:\path\to\driver\ppm_events_public.h(2304,23): warning C4200: nonstandard extension used: zero-sized array in struct/union

Signed-off-by: Gerald Combs <[email protected]>
@geraldcombs geraldcombs force-pushed the windows-plugin-path branch from 62669ee to 9f61775 Compare March 13, 2026 23:34
@geraldcombs
fix(plugin): Use UTF-8 paths on Windows
56cd265 
Document that plugin paths should be UTF-8 on Windows, and convert them
to UTF-16 internally.

In sinsp-example, add an application manifest that sets activeCodePage
to UTF-8 on Windows.

Signed-off-by: Gerald Combs <[email protected]>
@geraldcombs geraldcombs force-pushed the windows-plugin-path branch from 9f61775 to 56cd265 Compare March 15, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ekoops ekoops ekoops requested changes

@hbrueckner hbrueckner Awaiting requested review from hbrueckner

@terror96 terror96 Awaiting requested review from terror96

Requested changes must be addressed to merge this pull request.

Assignees

@terror96 terror96

@ekoops ekoops

Labels

approved area/libscap dco-signoff: yes do-not-merge/hold kind/bug Something isn't working kind/cleanup release-note-none size/M

Projects

Falco Roadmap
Status: In progress

Milestone

0.24.0

Development

Successfully merging this pull request may close these issues.

4 participants

@geraldcombs @terror96 @ekoops @poiana