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 idStr self-assignment #161

Merged
merged 1 commit into from
Mar 14, 2017
Merged

Fix idStr self-assignment #161

merged 1 commit into from
Mar 14, 2017

Conversation

turol
Copy link
Contributor

@turol turol commented Mar 14, 2017

Was calling memcpy with overlapping parameters which is inefficient,
undefined behavior and Valgrind complained about it.

Was calling memcpy with overlapping parameters which is inefficient,
undefined behavior and Valgrind complained about it.
@DanielGibson
Copy link
Member

Does it actually happen? If so, there's probably a bug in the caller, wouldn't make much sense..

@turol
Copy link
Contributor Author

turol commented Mar 14, 2017

https://github.com/dhewm/dhewm3/blob/master/neo/framework/Common.cpp#L2558
The code calls StripFileName on a variable and assigns the return value back to it. StripFileName returns reference to "this", resulting in self-assignment. The comment seems to indicate this is intentional.

@DanielGibson
Copy link
Member

ok, that was a workaround for a compilerbug.. I wonder if this breaks the workaround, after all it's an inline function and the compiler might notice that it'll do nothing and again optimize everything out..
but it's not like gcc 4.7 is very relevant anymore, so I'll just take that risk ;)

@DanielGibson DanielGibson merged commit 5da6374 into dhewm:master Mar 14, 2017
@DanielGibson
Copy link
Member

thanks a lot for the patch :)

@turol turol deleted the idstr-fix branch March 14, 2017 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants