Skip to content

Added default initialization to avoid garbage values #8973

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
3qupo wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Added default initialization to avoid garbage values #8973

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fd6857c
fix: initializing a variable of type FB_UINT64 with a null value
3qupo Aug 29, 2025
a6988a4
initializing a variable of type SINT64 with a null value
3qupo Aug 29, 2025
4e97b5c
changed the .ebb file, but the warning didn't go away
3qupo Sep 1, 2025
7ec3f4b
initializing a variable of type USHORT with a null value
3qupo Sep 1, 2025
a6db65e
initializing a pointer of type SLONG* nullptr
3qupo Sep 1, 2025
0fe3203
initializing a pointer of type UCHAR* nullptr
3qupo Sep 1, 2025
533aab5
initializing variables with zero and default value
3qupo Sep 1, 2025
64963c8
Delete skipfile.txt
3qupo Sep 4, 2025
bfa093b
the default value for trig_t has been added and initialized
3qupo Dec 13, 2025
f335f7c
the default value is 0 as there will be a recalculation
3qupo Dec 13, 2025
30810af
Merge branch 'master' into added_default_initialization_to_avoid_garb…
3qupo Mar 31, 2026
d84f8db
fix: initialize max_segment to USHRT_MAX for safe default
3qupo Apr 5, 2026
a2e9f22
fix: use fld_ranges as default for end_ranges initialization
3qupo Apr 5, 2026
cfcb213
refactor: move upper/lower declarations outside loops to avoid reinit…
3qupo Apr 5, 2026
f4c7ab2
fix: prevent undefined behavior by initializing usr, userSet and type…
3qupo Apr 5, 2026
7f2329f
fix: add check that trigger type was assigned a meaningful value
3qupo Apr 5, 2026
232b565
refactor: use proper TTypeId type for from_interp instead of generic …
3qupo Apr 5, 2026
d3d0952
revert: remove redundant type_found check as trig_none is caught by s…
3qupo Apr 5, 2026
ad8a3ad
Revert incorrect condition for ttype_none in CVT2_make_string2
3qupo Apr 14, 2026
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
1 change: 1 addition & 0 deletions src/burp/burp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,7 @@ enum att_type {
// Trigger types

enum trig_t {
trig_none = 0,
trig_pre_store = 1, // default
trig_pre_modify, // default
trig_post_erase // default
Expand Down
12 changes: 6 additions & 6 deletions src/burp/restore.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1608,7 +1608,7 @@ bool get_acl(BurpGlobals* tdgbl, const TEXT* owner_nm, ISC_QUAD* blob_id, ISC_QU

ULONG length = 0;
UCHAR item;
USHORT max_segment;
USHORT max_segment = 0;

@aafemt aafemt Mar 31, 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.

Wrong initialization. In this case USHRT_MAX is a safe assumption.

@dyemanov dyemanov Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wrong initialization. In this case USHRT_MAX is a safe assumption.

In backup.epp, max_segment is initialized with zero in the very same piece of code. If we have no reply for isc_info_blob_max_segment and blob total length is not zero, why should we imply length being USHRT_MAX as you suggest?

@aafemt aafemt Jul 3, 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.

Comment "Rdb sometimes gets the length messed up" suggests that total BLOB length can be evaluated wrongly. In this case using buffer of maximum allowed segment size is better.

I suspect that this piece of code exists only to avoid allocation of 64k memory unconditionally. It had sense in the previous century but not today.

@hvlad hvlad Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Its all makes no sense and is a waste of time. Vars gets its values from server response. There can be exactly one reason for any item to have no response - some fatal error. This is API contract. If it become broken, many places in code will fail.

Note, the title say: "...to avoid garbage values" - this code doesn't prevent any kind of garbage values.

Also, why max_segment gets default, while num_segments - not ? Looks like "slightly-pregnant", sorry...

@aafemt aafemt Jul 3, 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.

Because num_segments is not used at all. This routine still can handle only ACLs of max size 64k. Perhaps, @AlexPeshkoff missed it when fixed #2651.

ULONG num_segments;
const UCHAR* p = blob_info;

Expand Down Expand Up @@ -1783,7 +1783,7 @@ void get_array(BurpGlobals* tdgbl, burp_rel* relation, UCHAR* record_buffer)
SLONG fld_ranges[2 * MAX_DIMENSION];
SLONG slice_length = 0;
SLONG *range;
const SLONG* end_ranges;
const SLONG* end_ranges = nullptr;
Comment thread
dyemanov marked this conversation as resolved.
Outdated
scan_attr_t scan_next_attr;
skip_init(&scan_next_attr);
att_type attribute;
Expand Down Expand Up @@ -1851,7 +1851,7 @@ void get_array(BurpGlobals* tdgbl, burp_rel* relation, UCHAR* record_buffer)
SLONG last_element_dim[MAX_DIMENSION];
if (return_length != slice_length)
{
int upper, lower;
int upper = 0, lower = 0;
Comment thread
dyemanov marked this conversation as resolved.
Outdated
//
// Ugh! The full array wasn't returned and versions of gbak prior to
// V3.2I don't explicitly signal this. We must recompute the top
Expand Down Expand Up @@ -3386,7 +3386,7 @@ void get_data(BurpGlobals* tdgbl, burp_rel* relation, WriteRelationReq* req)
}
}

UCHAR* p;
UCHAR* p = nullptr;
if (tdgbl->gbl_sw_transportable)
{
if (get(tdgbl) != att_xdr_length)
Expand Down Expand Up @@ -9097,7 +9097,7 @@ bool get_db_creator(BurpGlobals* tdgbl)
att_type attribute;
scan_attr_t scan_next_attr;
TEXT usr[GDS_NAME_LEN];
SSHORT uType;
SSHORT uType = 0;
Comment thread
dyemanov marked this conversation as resolved.
bool userSet, typeSet;

userSet = typeSet = false;
Expand Down Expand Up @@ -9422,7 +9422,7 @@ bool get_trigger_old (BurpGlobals* tdgbl, burp_rel* relation)
* Get a trigger definition for a relation.
*
**************************************/
enum trig_t type;
enum trig_t type = trig_none;

@aafemt aafemt Mar 31, 2026 •
edited
Loading

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.

For completeness you should also add a check that a meaningful value was assigned below and throw an error if type is still trig_none.

@dyemanov dyemanov Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd better use std::optional rather than introduce trig_none for nothing useful.

att_type attribute;
TEXT name[GDS_NAME_LEN];
scan_attr_t scan_next_attr;
Expand Down
3 changes: 2 additions & 1 deletion src/dsql/DdlNodes.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ static void checkForeignKeyTempScope(thread_db* tdbb, jrd_tra* transaction,
{
AutoCacheRequest request(tdbb, drq_l_rel_info, DYN_REQUESTS);
QualifiedName masterRelName;
rel_t masterType, childType;
rel_t masterType = rel_persistent;

@aafemt aafemt Mar 31, 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.

This function is basically wrong and will fail in the case if the table has several foreign keys. It must be fixed, not polished.

rel_t childType = rel_persistent;

FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction)
RLC_M IN RDB$RELATION_CONSTRAINTS CROSS
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 @@ -973,7 +973,7 @@ USHORT CVT2_make_string2(const dsc* desc, TTypeId to_interp, UCHAR** address, Mo
**************************************/
UCHAR* from_buf;
USHORT from_len;
TTypeId from_interp;
USHORT from_interp = 0;
Comment thread
dyemanov marked this conversation as resolved.
Outdated

fb_assert(desc != NULL);
fb_assert(address != NULL);
Expand Down
2 changes: 1 addition & 1 deletion src/jrd/os/posix/unix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ bool PIO_header(thread_db* tdbb, UCHAR* address, unsigned length)
const auto dbb = tdbb->getDatabase();

unsigned i;
SINT64 bytes;
SINT64 bytes = 0;

PageSpace* const pageSpace = dbb->dbb_page_manager.findPageSpace(DB_PAGE_SPACE);
jrd_file* const file = pageSpace->file;
Expand Down
2 changes: 1 addition & 1 deletion src/remote/client/interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2011,7 +2011,7 @@ unsigned char* Attachment::getLocalInfo(UCharBuffer& info, unsigned int buffer_l
break;
}

FB_UINT64 value;
FB_UINT64 value = 0;
bool skip = false;

switch (*item)
Expand Down