Skip to content

Conversation

Rot127
Copy link
Collaborator

@Rot127 Rot127 commented Sep 30, 2025

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

It changes now to the following behavior:

Build type CAPSTONE_ASSERTION_WARNINGS Behavior
Debug Defined like print_warning() ; assert
Debug Undefined like assert
Release Defined like print_warning()
Release Undefined like {}

This also fixes the inconsistent (buggy) behavior from before.
The variants of CS_ASSERT_RET(_VAL) did not return if
the assertions were disabled (release build).
So the semantics were different.

Test plan

Added and old ones adjusted.

Closing issues

closes #2791

@github-actions github-actions bot added CS-core-files auto-sync Github-files Github related files labels Sep 30, 2025
@Rot127 Rot127 marked this pull request as draft September 30, 2025 11:03
@Rot127 Rot127 force-pushed the assert branch 17 times, most recently from b247ccc to ac78ba4 Compare September 30, 2025 16:06
@Rot127
Copy link
Collaborator Author

Rot127 commented Sep 30, 2025

@hgarrereyn If you find time I would grateful if you could check the macro changes as well.
Specifically if you think they are intuitive as user.

@Rot127 Rot127 marked this pull request as ready for review September 30, 2025 16:45
It changes now to the following behavior:

- Debug && !defined(CAPSTONE_ASSERTION_WARNINGS)
  - `assert(expr)` code is included, warning is not printed if hit.
- Debug && defined(CAPSTONE_ASSERTION_WARNINGS)
  - `assert(expr)` code is included, warning is printed if hit.
- Release && !defined(CAPSTONE_ASSERTION_WARNINGS)
  - `assert(expr)` code is not included, warning is not printed if hit.
- Release && defined(CAPSTONE_ASSERTION_WARNINGS)
  - `assert(expr)` code is not included, warning is printed if hit.

This also fixes the inconsistent (buggy) behavior from before.
The variants of CS_ASSERT_RET(_VAL) did not return if
the assertions were disabled (release build).
So the semantics were different.
@Rot127
Copy link
Collaborator Author

Rot127 commented Oct 4, 2025

@jiegec Can you check a second time when you find time?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CS-core-files auto-sync Github-files Github related files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No error handling in cs_reg_name
2 participants