Skip to content

[GEN][ZH] Demote throw to assert in AsciiString::format_va, UnicodeString::format_va #835

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

Merged
merged 3 commits into from
May 10, 2025

Conversation

xezon
Copy link

@xezon xezon commented May 9, 2025

This change demotes the unnecessary throw to debug assert in AsciiString::format_va and UnicodeString::format_va. Unnecessary throw because this format error is not necessarily a fatal error that needs to terminate the process. The program may just run fine with this error. The debug assert is enough to see the error.

Original

image

This change

image

@xezon xezon added this to the Stability fixes milestone May 9, 2025
@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Refactor Improves the structure of the code, with negligible changes in function. Stability Concerns stability of the runtime labels May 9, 2025
throw ERROR_OUT_OF_MEMORY;
set(buf);
validate();
const int result = _vsnprintf(buf, sizeof(buf)/sizeof(WideChar)-1, format, args);

Choose a reason for hiding this comment

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

sizeof(char)

Copy link
Author

Choose a reason for hiding this comment

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

Ups. Fixed.

const int result = _vsnprintf(buf, sizeof(buf)/sizeof(WideChar)-1, format, args);
if (result >= 0)
{
set(buf);

Choose a reason for hiding this comment

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

can we set buf[MAX_FORMAT_BUF_LEN-1] = 0; while we are at it?

Copy link
Author

Choose a reason for hiding this comment

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

Regarding _vsnprintf & _snprintf, I planned to do another change.

@xezon xezon requested a review from helmutbuhler May 10, 2025 07:05
Copy link

@helmutbuhler helmutbuhler left a comment

Choose a reason for hiding this comment

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

Looks good

@xezon xezon merged commit 0e1bc63 into TheSuperHackers:main May 10, 2025
18 checks passed
@xezon xezon deleted the xezon/refactor-formatva branch May 10, 2025 10:41
@xezon xezon added Gen Relates to Generals ZH Relates to Zero Hour labels May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker Refactor Improves the structure of the code, with negligible changes in function. Stability Concerns stability of the runtime ZH Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants