Skip to content

Make ROOT's string functions private to ROOT - #23150

Draft
hageboeck wants to merge 6 commits into
root-project:masterfrom
hageboeck:builtin_c_lib
Draft

Make ROOT's string functions private to ROOT#23150
hageboeck wants to merge 6 commits into
root-project:masterfrom
hageboeck:builtin_c_lib

Conversation

@hageboeck

@hageboeck hageboeck commented Aug 26, 2026

Copy link
Copy Markdown
Member

With a vision to possibly remove ROOT's C-string functions, make them private to ROOT.

This library consists of three (for now) public headers:

  • snprintf.h: Should be replaced with the C99 version.
  • strlcpy.h: Like in libbsd. Try using it only in ROOT's .cxx files, so no public usage is visible from outside. Later, replace with standard functions.
  • strtok.h: Defines R__STRTOK_R. Only used in .cxx, so easy to hide. Could consider replacing it with standard functions later.

Here, the first steps are taken to making the library private, so that it can either be replaced or removed without problems:

  • Replace ROOT's snprintf with the C99 version.
  • Remove any use of the above headers in ROOT's headers
  • Remove implementation and headers from libCore
  • Pack headers and implementation into a static library that is used exclusively with target_link_libraries(... PRIVATE), and with symbol visibility=hidden

@hageboeck hageboeck self-assigned this Aug 26, 2026
@hageboeck
hageboeck force-pushed the builtin_c_lib branch 2 times, most recently from 9cebecd to 2385a9c Compare August 26, 2026 11:19
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown

Test Results

    21 files      21 suites   3d 8h 55m 22s ⏱️
 3 838 tests  3 837 ✅ 0 💤 1 ❌
72 563 runs  72 561 ✅ 1 💤 1 ❌

For more details on these failures, see this check.

Results for commit 17ac674.

♻️ This comment has been updated with latest results.

Comment on lines +39 to +40
std::memcpy(_label, other._label, sizeof(_label));
_label[255] = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not quite the same operation as before, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Indeed, but it "correctly" copies the array (i.e. uses its actual size instead of a hardcoded one), and ensures that the string is always terminated.
The motivation is to remove any uses of ROOT's string functions from ROOT headers, so a replacement was needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But indeed, I maybe shouldn't hardcode 255 if I remove the hardcoded number in the line above. 🙂

@hageboeck
hageboeck force-pushed the builtin_c_lib branch 3 times, most recently from e5fcbae to da5daa4 Compare August 28, 2026 10:42
@hageboeck hageboeck changed the title First step of making the builtin libbsd private to ROOT Make the builtin libbsd string functions private to ROOT Aug 28, 2026
@hageboeck hageboeck added the clean build Ask CI to do non-incremental build on PR label Sep 1, 2026
- Add a missing include
- Remove usage of strlcpy from all RooFit headers
snprintf is standardised, so there is no more need to bring a custom
implementation.
@hageboeck
hageboeck force-pushed the builtin_c_lib branch 2 times, most recently from 0236682 to b16128d Compare September 1, 2026 08:51
Move string functions that originated in libbsd into a private library
that's only compiled into ROOT's *.cxx files. Before, they were in Core,
and the corresponding headers were visible outside of ROOT.

Remove the headers from install set and modulemap.
@hageboeck hageboeck changed the title Make the builtin libbsd string functions private to ROOT Make ROOT's string functions private to ROOT Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants