Skip to content

Remove undefined behavior from QMessageLogContext#1399

Merged
ahayzen-kdab merged 1 commit intoKDAB:mainfrom
jnbooth:remove-logging-undefined-behavior
Mar 27, 2026
Merged

Remove undefined behavior from QMessageLogContext#1399
ahayzen-kdab merged 1 commit intoKDAB:mainfrom
jnbooth:remove-logging-undefined-behavior

Conversation

@jnbooth
Copy link
Copy Markdown
Contributor

@jnbooth jnbooth commented Jan 28, 2026

According to the safety documentation for CStr::from_ptr, the pointer must be non-null. Currently, the functions for QMessageLogContext do not check for null pointers. This PR adds those checks, removing the undefined behavior.

Note: cxx-qt-lib's qtlogging code is noticeably messy. In addition to the above undefined behavior, it has extraneous unsafe specifiers and a pointless const assertion that was probably supposed to occur on the C++ side, and its functions are in the global namespace rather than rust::cxxqtlib1. It also solely exposes undocumented internal functionality and doesn't respect category disabling. I made #1398 to serve as an alternative for it.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ee1849d) to head (14aade7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1399   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           75        75           
  Lines        13457     13457           
=========================================
  Hits         13457     13457           

☔ 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.

@ahayzen-kdab ahayzen-kdab added the ⏮️ backport-candidate Change which could be backported to the stable series label Mar 27, 2026
@ahayzen-kdab ahayzen-kdab force-pushed the remove-logging-undefined-behavior branch from 771fd43 to 14aade7 Compare March 27, 2026 10:17
Copy link
Copy Markdown
Collaborator

@ahayzen-kdab ahayzen-kdab left a comment

Choose a reason for hiding this comment

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

LGTM :-) Good catch on the CStr::from_ptr, and yes those unsafe are not needed probably a misunderstanding how CXX requires unsafe when pointers are used in arguments but it doesn't matter for return types :-)

And yes adding namespaces on the C++ side would be good and that const assert is already on the C++ side and not really checking anything else, so feel free to tidy up in another PR :-)

@ahayzen-kdab ahayzen-kdab enabled auto-merge March 27, 2026 10:20
@ahayzen-kdab ahayzen-kdab added this pull request to the merge queue Mar 27, 2026
Merged via the queue into KDAB:main with commit 1b91ce6 Mar 27, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⏮️ backport-candidate Change which could be backported to the stable series

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants