Skip to content

Added a null-terminating character to the character list when scanning a number. #2263

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

Merged
merged 1 commit into from
Jan 3, 2017
Merged

Added a null-terminating character to the character list when scanning a number. #2263

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 5 additions & 11 deletions lib/Runtime/Debug/TTSerialize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,9 @@ namespace TTD
charList.Add(c);
}

// Null-terminate the list before we try to use the buffer as a string.
charList.Add(_u('\0'));

bool likelyint; //we don't care about this just want to know that it is convertable to a number
const char16* end;
const char16* start = charList.GetBuffer();
Expand Down Expand Up @@ -963,8 +966,8 @@ namespace TTD
return NSTokens::ParseTokenKind::Error;
}

//convert this number to get the length of the string (not including "")
charList.Add(_u('\0'));
// Convert this number to get the length of the string (not including ""),
// charList is already null-terminated by the call to ScanNumber.
Copy link
Collaborator

@obastemur obastemur Dec 21, 2016

Choose a reason for hiding this comment

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

At the end of this method, charList is recreated with no null ending. I didn't follow the whole call chain but better we add null ending there too?

Copy link
Contributor

@mrkmarron mrkmarron Dec 21, 2016

Choose a reason for hiding this comment

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

In general we don't assume that the charList buffers are null terminated, the caller should insert a null terminator if needed, numbers are a special case. Specifically, adding a null terminator here will break the string parsing.

Copy link
Collaborator

@obastemur obastemur Dec 21, 2016

Choose a reason for hiding this comment

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

Actually for DEBUG purposes, we might add a check if null_ending was ever added to charList and assert that on getBuffer call. This approach would give us an early warning on the consumer end.

@digitalinfinity was asking for a test implementation here, maybe this could be the approach to take ?

Copy link
Contributor Author

@kfarnung kfarnung Dec 21, 2016

Choose a reason for hiding this comment

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

One thought I had was to add an assert to each location where we get the buffer in this code.

Something like:

TTDAssert(charList.Item(charList.Count() - 1) == '\0', "Char list must be null terminated");

uint32 length = (uint32)this->ReadUIntFromCharArray(charList.GetBuffer());

//read the lead "\""
Expand Down Expand Up @@ -1226,7 +1229,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::Number, "Error in parse.");

this->m_charListOpt.Add(_u('\0'));
uint64 uval = this->ReadUIntFromCharArray(this->m_charListOpt.GetBuffer());
TTDAssert(uval <= BYTE_MAX, "Error in parse.");

Expand All @@ -1250,7 +1252,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::Number, "Error in parse.");

this->m_charListOpt.Add(_u('\0'));
int64 ival = this->ReadIntFromCharArray(this->m_charListOpt.GetBuffer());
TTDAssert(INT32_MIN <= ival && ival <= INT32_MAX, "Error in parse.");

Expand All @@ -1264,7 +1265,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::Number, "Error in parse.");

this->m_charListOpt.Add(_u('\0'));
uint64 uval = this->ReadUIntFromCharArray(this->m_charListOpt.GetBuffer());
TTDAssert(uval <= UINT32_MAX, "Error in parse.");

Expand All @@ -1278,7 +1278,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::Number, "Error in parse.");

this->m_charListOpt.Add(_u('\0'));
return this->ReadIntFromCharArray(this->m_charListOpt.GetBuffer());
}

Expand All @@ -1289,7 +1288,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::Number, "Error in parse.");

this->m_charListOpt.Add(_u('\0'));
return this->ReadUIntFromCharArray(this->m_charListOpt.GetBuffer());
}

Expand Down Expand Up @@ -1324,7 +1322,6 @@ namespace TTD
{
TTDAssert(tok == NSTokens::ParseTokenKind::Number, "Error in parse.");

this->m_charListOpt.Add(_u('\0'));
res = this->ReadDoubleFromCharArray(this->m_charListOpt.GetBuffer());

break;
Expand All @@ -1341,7 +1338,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::Address, "Error in parse.");

this->m_charListOpt.Add(_u('\0')); //add terminator
return (TTD_PTR_ID)this->ReadUIntFromCharArray(this->m_charListOpt.GetBuffer());
}

Expand All @@ -1352,7 +1348,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::LogTag, "Error in parse.");

this->m_charListOpt.Add(_u('\0')); //add terminator
return (TTD_LOG_PTR_ID)this->ReadUIntFromCharArray(this->m_charListOpt.GetBuffer());
}

Expand All @@ -1363,7 +1358,6 @@ namespace TTD
NSTokens::ParseTokenKind tok = this->Scan(this->m_charListOpt);
TTDAssert(tok == NSTokens::ParseTokenKind::EnumTag, "Error in parse.");

this->m_charListOpt.Add(_u('\0')); //add terminator
uint64 tval = this->ReadUIntFromCharArray(this->m_charListOpt.GetBuffer());
TTDAssert(tval <= UINT32_MAX, "Error in parse.");

Expand Down