Skip to content
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.

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

Fix some Coverity warnings in String API #18281

Merged
merged 1 commit into from
Apr 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 3 additions & 10 deletions core/string_db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,21 +164,14 @@ void StringName::operator=(const StringName &p_name) {
_data = p_name._data;
}
}
/* was inlined
StringName::operator String() const {

if (_data)
return _data->get_name();

return "";
}
*/
StringName::StringName(const StringName &p_name) {

ERR_FAIL_COND(!configured);
_data = NULL;
if (p_name._data && p_name._data->refcount.ref()) {

ERR_FAIL_COND(!configured);

if (p_name._data && p_name._data->refcount.ref()) {
_data = p_name._data;
}
}
Expand Down
1 change: 1 addition & 0 deletions core/string_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class StringName {
_Data() {
cname = NULL;
next = prev = NULL;
idx = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

Coverity also reports:

Non-static class member field refcount.count is not initialized in this constructor nor in any functions that it calls.

But I'm not sure how/if it should be fixed. The code that actually uses _data always seems to do _data->refcount.init();, I guess it could be made here too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or should we make SafeRefCount initialize its count to 0?

hash = 0;
}
};
Expand Down
28 changes: 9 additions & 19 deletions core/ustring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ String String::num_uint64(uint64_t p_num, int base, bool capitalize_hex) {
c[chars] = 0;
n = p_num;
do {
int mod = ABS(n % base);
int mod = n % base;
if (mod >= 10) {
char a = (capitalize_hex ? 'A' : 'a');
c[--chars] = a + (mod - 10);
Expand Down Expand Up @@ -1550,8 +1550,7 @@ String::String(const StrRange &p_range) {

int String::hex_to_int(bool p_with_prefix) const {

int l = length();
if (p_with_prefix && l < 3)
if (p_with_prefix && length() < 3)
return 0;

const CharType *s = ptr();
Expand All @@ -1560,17 +1559,13 @@ int String::hex_to_int(bool p_with_prefix) const {

if (sign < 0) {
s++;
l--;
if (p_with_prefix && l < 2)
return 0;
}

if (p_with_prefix) {
if (s[0] != '0' || s[1] != 'x')
return 0;
s += 2;
l -= 2;
};
}

int hex = 0;

Expand All @@ -1596,8 +1591,7 @@ int String::hex_to_int(bool p_with_prefix) const {

int64_t String::hex_to_int64(bool p_with_prefix) const {

int l = length();
if (p_with_prefix && l < 3)
if (p_with_prefix && length() < 3)
return 0;

const CharType *s = ptr();
Expand All @@ -1606,17 +1600,13 @@ int64_t String::hex_to_int64(bool p_with_prefix) const {

if (sign < 0) {
s++;
l--;
if (p_with_prefix && l < 2)
return 0;
}

if (p_with_prefix) {
if (s[0] != '0' || s[1] != 'x')
return 0;
s += 2;
l -= 2;
};
}

int64_t hex = 0;

Expand Down Expand Up @@ -3478,21 +3468,21 @@ bool String::is_valid_hex_number(bool p_with_prefix) const {

if (p_with_prefix) {

if (len < 2)
if (len < 3)
return false;
if (operator[](from) != '0' || operator[](from + 1) != 'x') {
return false;
};
}
from += 2;
};
}

for (int i = from; i < len; i++) {

CharType c = operator[](i);
if ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'))
continue;
return false;
};
}

return true;
};
Expand Down