Skip to content

Add asserts in trace - #9118

Open
Foxpunk wants to merge 9 commits into
FirebirdSQL:masterfrom
craftmaster1231:add_asserts_in_trace
Open

Add asserts in trace#9118
Foxpunk wants to merge 9 commits into
FirebirdSQL:masterfrom
craftmaster1231:add_asserts_in_trace

Conversation

@Foxpunk

@Foxpunk Foxpunk commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Problem

Several switch statements over enum-like fields, and return values of
time(), fseek() and snprintf(), were not checked, which could lead to
silent incorrect behavior.

Fix

Added explicit default: fb_assert(false) branches to switch
statements to catch unexpected values early.
Saved the return values of time(), fseek() and snprintf() into
[[maybe_unused]] variables and added fb_assert checks on them.

@hvlad

hvlad commented Aug 7, 2026

Copy link
Copy Markdown
Member

Problem

Several switch statements over enum-like fields, and return values of time(), fseek() and snprintf(), were not checked, which could lead to silent incorrect behavior.

Fix

Added explicit default: fb_assert(false) branches to switch
statements to catch unexpected values early.

You should better look at the code and get understanding what it does.

Saved the return values of time(), fseek() and snprintf() into
[[maybe_unused]] variables and added fb_assert checks on them.

How this visual spam prevents "silent incorrect behavior" ? Sorry, can't resist.

@Foxpunk

Foxpunk commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@hvlad I can drop the fb_assert(false) default branches from the switch
statements. I just thought it might help during testing, but I don't
have a strong enough argument for keeping them.

Can I keep the fb_assert checks on the time()/fseek()/snprintf() return
values, since those actually verify something concrete?

I'll also clean up the PR description to be more to the point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants