Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Zakk0n wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66bfb4f
Null pointers done
Zakk0n Aug 27, 2025
fa01cd9
Unserialized and uninitialized done
Zakk0n Sep 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/common/DynamicStrings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@aafemt aafemt Mar 30, 2026

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.

break;
memcpy(string, reinterpret_cast<const char*>(from[1]), from[0]);
string += *from++;
*string++ = '\0';
Expand All @@ -97,6 +99,8 @@ unsigned makeDynamicStrings(unsigned length, ISC_STATUS* const dst, const ISC_ST
case isc_arg_sql_state:
fb_assert(string);
*to++ = (ISC_STATUS)(IPTR) string;
if (!string)
break;
strcpy(string, reinterpret_cast<const char*>(*from));
string += strlen(string);
string++;
Expand Down
2 changes: 1 addition & 1 deletion src/dsql/DdlNodes.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ void defineComputed(DsqlCompilerScratch* dsqlScratch, RelationSourceNode* relati
// Save the size of the field if it is specified.
dsc saveDesc;
saveDesc.dsc_dtype = 0;
bool saveCharSetIdSpecified;
bool saveCharSetIdSpecified = false;

if (field && field->dtype)
{
Expand Down
2 changes: 2 additions & 0 deletions src/gpre/c_cxx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@aafemt aafemt Mar 30, 2026

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.

PATTERN_expand((USHORT) column, pattern1, &args);

set_sqlcode(action, column);
Expand Down
3 changes: 2 additions & 1 deletion src/gpre/obj_cxx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3368,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

@aafemt aafemt Mar 30, 2026

Choose a reason for hiding this comment

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

But what's else?

printa(column, "%s = %s->startTransaction(%s, %d, fb_tpb_%d);",
trans->tra_handle ? trans->tra_handle : gpreGlob.transaction_name,
trans->tra_tpb->tpb_database->dbb_name->sym_string, vector,
trans->tra_tpb->tpb_length, trans->tra_tpb->tpb_ident);
Expand Down
6 changes: 4 additions & 2 deletions src/gpre/pat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,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

@aafemt aafemt Mar 30, 2026

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?

long_value = args->pat_long1;
long_flag = true;
break;

case L2:
long_value = args->pat_long2;
if (args && args->pat_long2)
long_value = args->pat_long2;
long_flag = true;
break;

Expand Down
4 changes: 2 additions & 2 deletions src/jrd/IntlManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@aafemt aafemt Mar 30, 2026

Choose a reason for hiding this comment

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

nullptr, please.

if (module &&
(objModule = configFile.findParameter("intl_module", module->value.c_str())))
{
if (!objModule->sub)
if (!objModule || !objModule->sub)
fatal_exception::raiseFmt("Missing parameters for intl_module %s\n", module->value.c_str());

const ConfigFile::Parameter* fname = objModule->sub->findParameter("filename");
Expand Down
2 changes: 1 addition & 1 deletion src/jrd/cvt2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ int CVT2_blob_compare(const dsc* arg1, const dsc* arg2, DecimalStatus decSt)
*
**************************************/

SLONG l1, l2;
SLONG l1 = NULL, l2 = NULL;
USHORT ttype2;
int ret_val = 0;

Expand Down
6 changes: 6 additions & 0 deletions src/jrd/lck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,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)
return NULL;

Lock* match = (*att->att_compatibility_table)[hash_value];
if (!match)
return NULL;
Expand Down Expand Up @@ -1333,6 +1336,9 @@ 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

@aafemt aafemt Mar 30, 2026

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.


if (level < first->lck_physical)
{
if (dbb->lockManager()->convert(tdbb, statusVector, first->lck_id, level, LCK_NO_WAIT,
Expand Down
10 changes: 6 additions & 4 deletions src/jrd/replication/Applier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,10 +998,12 @@ 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)
blob->BLB_put_segment(tdbb, data, length);
else
blob->BLB_close(tdbb);
if (blob){
Copy link
Copy Markdown
Contributor

@aafemt aafemt Mar 30, 2026

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.

if (length)
blob->BLB_put_segment(tdbb, data, length);
else
blob->BLB_close(tdbb);
}
}

void Applier::executeSql(thread_db* tdbb,
Expand Down
4 changes: 2 additions & 2 deletions src/remote/server/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6129,10 +6129,10 @@ bool rem_port::sendInlineBlob(PACKET* sendL, Rtr* rtr, SQUAD blobId, ULONG maxSi
if (status.getState() & IStatus::STATE_ERRORS)
return false;

bool segmented;
bool segmented = false;
ULONG num_segments;
ULONG max_segment;
FB_UINT64 total_length;
FB_UINT64 total_length = 0u;

ClumpletReader p(ClumpletReader::InfoResponse, info, sizeof(info));
for (; !p.isEof(); p.moveNext())
Expand Down