Skip to content

Conversation

@uno1982
Copy link
Contributor

@uno1982 uno1982 commented May 3, 2025

Fixes: #106012

It appears the function return from the windows API included a null termination?
I've commented this specific segment of code with a safety to handle this.
There was quite a bit of changes between 4.4 and 4.5-Dev around print() and print_rich() that created some difficulty in hunting the exact cause of the editor output hang while the console exe continued to function

This fix addresses the length, output and concat, issues.

Before:
image

After:
image

@uno1982 uno1982 requested a review from a team as a code owner May 3, 2025 09:53
@AThousandShips AThousandShips added this to the 4.5 milestone May 3, 2025
@AThousandShips AThousandShips changed the title Fix: Windows OS.get_unique_id() null termination issue Fix Windows OS.get_unique_id() null termination issue May 3, 2025
@uno1982 uno1982 force-pushed the correct-get_unique_id-windows-null-termination branch from 21f05d2 to eab57c5 Compare May 3, 2025 11:01
@uno1982 uno1982 requested a review from AThousandShips May 3, 2025 11:23
@uno1982
Copy link
Contributor Author

uno1982 commented May 21, 2025

Just following up @AThousandShips I think we are good here right? OP of the initial report suggested the issue was resolved with this PR as well. :) Let me know if I can assist further. 👍

@AThousandShips
Copy link
Member

I think it looks alright, but I'm not experienced on the Windows side so this needs a review by that team

@akien-mga akien-mga requested a review from bruvzg May 21, 2025 16:51
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Windows docs use:
_tprintf(TEXT("Profile Guid = %s\n"), HwProfInfo.szHwProfileGuid);
as a usage example, so a null terminated string seems to be expected.

But to be 100% safe, we can check both null termination and buffer size:

return String::ascii(Span<char>(HwProfInfo.szHwProfileGuid, strnlen(HwProfInfo.szHwProfileGuid, HW_PROFILE_GUIDLEN)));

@akien-mga
Copy link
Member

Could you squash the commits? See PR workflow for instructions.

@uno1982 uno1982 requested review from a team as code owners May 22, 2025 02:17
@AThousandShips AThousandShips removed request for a team May 22, 2025 07:54
@uno1982 uno1982 force-pushed the correct-get_unique_id-windows-null-termination branch 2 times, most recently from 8994c87 to 3e3e631 Compare May 23, 2025 18:59
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Apologies for the delay - I had a couple review comments queued which I forgot to submit.

use ascii for encoding-neutral
check buffer size as well
@uno1982 uno1982 force-pushed the correct-get_unique_id-windows-null-termination branch from d675e10 to d79258c Compare May 23, 2025 20:05
@Repiteo Repiteo merged commit e2d244b into godotengine:master May 26, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented May 26, 2025

Thanks!

@uno1982 uno1982 deleted the correct-get_unique_id-windows-null-termination branch May 27, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows OS.get_unique_id() bug

5 participants