Skip to content

Added multiple NULL and nullptr checks#8962

Open
Zakk0n wants to merge 1 commit intoFirebirdSQL:masterfrom
Zakk0n:nullpointers
Open

Added multiple NULL and nullptr checks#8962
Zakk0n wants to merge 1 commit intoFirebirdSQL:masterfrom
Zakk0n:nullpointers

Conversation

@Zakk0n
Copy link
Copy Markdown

@Zakk0n Zakk0n commented Mar 29, 2026

This PR is a part of #8938

Copy link
Copy Markdown
Contributor

@aafemt aafemt left a comment

Choose a reason for hiding this comment

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

Mechanical "fixing" of AI-produced warnings, no code analyze performed. Too bad even for students.

@@ -87,6 +87,8 @@ unsigned makeDynamicStrings(unsigned length, ISC_STATUS* const dst, const ISC_ST
case isc_arg_cstring:
fb_assert(string);
*to++ = (ISC_STATUS)(IPTR) string;
if (!string)
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.

This is a dead code. The loop above makes sure that string is not nullptr.

@@ -3367,7 +3368,8 @@ static void gen_t_start( const act* action, int column)

if (trans->tra_db_count == 1)
{
printa(column, "%s = %s->startTransaction(%s, %d, fb_tpb_%d);",
if (trans->tra_tpb)
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.

And what's else? Don't start the transaction at all?..

@@ -990,15 +998,15 @@ void Applier::storeBlob(thread_db* tdbb, TraNumber traNum, bid* blobId,
fb_assert(blob->blb_flags & BLB_temporary);
fb_assert(!(blob->blb_flags & BLB_closed));

if (length)
if (blob && length)
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.

...and if blob is nullptr - still get AV on BLB_close call. Instead else branch must be added to if in line 983 with bugcheck logging and bail.

@@ -1069,6 +1049,9 @@ static Lock* hash_get_lock(Lock* lock, USHORT* hash_slot, Lock*** prior)

// if no collisions found, we're done

if (!att->att_compatibility_table)
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.

att_compatibility_table is surely initialized in line 1042. Another dead code.

@@ -1353,10 +1336,13 @@ static USHORT internal_downgrade(thread_db* tdbb, CheckStatusWrapper* statusVect

// if we can convert to that level, set all identical locks as having that level

if (!first || !first->lck_physical)
return NULL;
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.

Not NULL but a proper LCK_* constant here. Which one is "proper"? That's the question. Investigation also can find out if cases when first is nullptr are possible at all. BTW, lck_physical is not a pointer.

@sim1984
Copy link
Copy Markdown
Contributor

sim1984 commented Mar 30, 2026

How is this PR different from #8964 ?

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