Skip to content

Fix static analysis warnings - #9216

Open
Ev3nt wants to merge 8 commits into
FirebirdSQL:masterfrom
Ev3nt:fix-static-analysis-warnings
Open

Ev3nt wants to merge 8 commits into
FirebirdSQL:masterfrom
Ev3nt:fix-static-analysis-warnings

Conversation

@Ev3nt

@Ev3nt Ev3nt commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a set of warnings reported by the Svacer static analyzer. (Duplicate for unstable branch)

Changes

  • Initialize previously uninitialized fields
  • Fix possible division by zero
  • Remove unused fields
  • Handle the return value of ftell() instead of ignoring errors
  • Remove redundant comparison with a null pointer
  • Fix null pointer dereference

Each fix is in a separate commit, so they can be reviewed one by one.

Behavior impact

These changes address potential defects and are not intended to alter expected behavior.

// mark the index as utilized for the purposes of this compile
idx->idx_runtime_flags |= idx_used;

const ULONG impure = csb ? csb->allocImpure<impure_inversion>() : 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you think that this comparison is redundant, you must make it sure by replacing of a pointer with a reference through whole call stack. The same with m_creation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just look at the beginning of the function; it contains fb_assert(csb), and csb->csb_g_flags is accessed without any check.

As for m_creation, it is used only in one place:

IndexCreateTask task(tdbb, dbb->dbb_permanent, &creation);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fb_assert is a soft trap, hot hard guarantee. The problem is that Firebird QA use a release build where fb_assert is no-op. As an overall result you just add another place for null pointer dereference and crash.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

and csb->csb_g_flags is accessed without any check.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is something to fix, not excuse.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fb_assert is a soft trap, hot hard guarantee. The problem is that Firebird QA use a release build where fb_assert is no-op. As an overall result you just add another place for null pointer dereference and crash.

Ussaly, a fb_assert(csb) call implies that it is the programmer’s responsibility to ensure that csb exists at the time this function is called.

@aafemt aafemt Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Exactly as I said: use a reference for that. Reference produces a clean compilation error, not some runtime abend.

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.

3 participants