Skip to content

Added default initialization to avoid garbage values + extra NULL checks#8964

Open
Zakk0n wants to merge 2 commits intoFirebirdSQL:masterfrom
Zakk0n:Unserialized_Uninitialized
Open

Added default initialization to avoid garbage values + extra NULL checks#8964
Zakk0n wants to merge 2 commits intoFirebirdSQL:masterfrom
Zakk0n:Unserialized_Uninitialized

Conversation

@Zakk0n
Copy link
Copy Markdown

@Zakk0n Zakk0n commented Mar 29, 2026

This PR is a part of #8938

@@ -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 actually a dead code. The loop above make sure that string is never nullptr if status vector contains strings.

@@ -2668,6 +2668,8 @@ static void gen_put_segment( const act* action, int column)
args.pat_ident1 = blob->blb_len_ident;
args.pat_ident2 = blob->blb_buff_ident;
args.pat_string1 = global_status_name;
args.pat_long1 = NULL;
args.pat_long2 = 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.

Assign NULL to integral types is a bad idea because some compilers define NULL as a (void*)0 and issue a warning on such assignment.

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

But what's else?

@@ -355,12 +364,14 @@ void PATTERN_expand( USHORT column, const TEXT* pattern, PAT* args)
break;

case L1:
long_value = args->pat_long1;
if (args && args->pat_long1)
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.

Again, what's else and why only these two cases? Did you look for cases when args can be nullptr?

blob->BLB_put_segment(tdbb, data, length);
else
blob->BLB_close(tdbb);
if (blob){
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.

Bracket on a separate line, please, and writing of a diagnostic to the log in blob == nullptr branch to prevent the bug being unnoticed.

@@ -458,11 +458,11 @@ bool IntlManager::initialize()
string configInfo;

const ConfigFile::Parameter* module = ch->sub->findParameter("intl_module");
const ConfigFile::Parameter* objModule;
const ConfigFile::Parameter* objModule = 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.

nullptr, please.

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

Meaningful for this case LCK_* constant here, please. Which value would be a meaningful? That's the question.

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