-
Notifications
You must be signed in to change notification settings - Fork 81
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
Sanitizer fixes and plProduct static init fix #1585
Conversation
Thanks - the address sanitizer stuff was bugging me for a while. Hopefully this is the same issues I was seeing. |
56c4a36
to
cfd9402
Compare
I have dropped the change to plDetectorLog and instead made more of the plProduct strings use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look good to me overall now. (I have no particular opinions on the things @Hoikas commented)
Looks like we have a test failing on macOS. |
The pnUUID test was failing on the string comparison due to case differences. I've worked around that for now by using It seems that Linux and Windows stringify plUUID with lowercase letters. On macOS it's using uppercase letters. I suspect the plUUID |
This should avoid some static initialization ordering issues that were cropping up with ST::string construction.
Okay, this should be good to go now. I opted to enforce that stringified UUIDs are lowercase for consistency with existing Windows code. |
Co-Authored-By: dgelessus <[email protected]>
Co-Authored-By: dgelessus <[email protected]>
I'll toss in that just for "technically correct"-ness - UUIDs should always be compared case insensitively. https://datatracker.ietf.org/doc/html/rfc4122
Apple's implementation looks out of line from the spec (I'd be curious as to why) as they output in upper case - but not in a way that should actually impact UUID use. Something to keep in mind for any sort of comparisons we do with UUIDs though. (Backtracking through the UUID specifications is kind of weird in since their use was spread so wide, by my understanding is RFC4122 restates the UUID specification.) |
Any comparisons of UUIDs should be done bitwise by the plUUID class, this only came to light because I added a unit test that checked the stringified output against a known value. |
plDetectorLog was initializing before plProduct had initialized its static strings, leading to some weird stuff where the User Data Folder path was missing "Uru Live"
Was running with AddressSanitizer to try to track down some GL Pipeline issues, and ended up tracking down a bunch of other issues and warnings first.
memcpy
is a built-in now so there should be no speed benefit to doing casting.fRegisteredForTime
(and a few other values) were never being zero initialized in plTransitionMgrfBuffer
is created withnew[]
but was being released withdelete