The Wayback Machine - https://web.archive.org/web/20201216142316/https://github.com/microsoft/Detours/pull/112
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

Replace DWORDXX with IMAGE_THUNK_DATAXX. #112

Merged
merged 1 commit into from Sep 5, 2020
Merged

Conversation

@lironzua
Copy link
Contributor

@lironzua lironzua commented Aug 2, 2020

Since both of the types have the same capacity in the same arch, they both work. But IMAGE_THUNK_DATAXX is the right struct for this code and I believe to make the code more readable and descriptive.

@microsoft-cla
Copy link

@microsoft-cla microsoft-cla bot commented Aug 2, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@bgianfo bgianfo left a comment

Hey @lironzua would you mind rebasing this on latest master so we can make sure it doesn't break the new build validation that was added before merge?

@lironzua
Copy link
Contributor Author

@lironzua lironzua commented Sep 4, 2020

@bgianfo done

@bgianfo
Copy link
Member

@bgianfo bgianfo commented Sep 4, 2020

Looks like your branch for patch-4 is still behind ~20 commits, missing the new PR Valdation.
See: https://github.com/lironzua/Detours/commits/patch-4

@lironzua lironzua force-pushed the lironzua:patch-4 branch from 5130f95 to 65dd6c1 Sep 4, 2020
@lironzua
Copy link
Contributor Author

@lironzua lironzua commented Sep 4, 2020

My bad, I updated something else by mistake. I rebased again here. Also, what PR validation did you mean? LMK if anything is missing.

Thanks

@bgianfo
Copy link
Member

@bgianfo bgianfo commented Sep 4, 2020

Also, what PR validation did you mean? LMK if anything is missing.

Looks like it caught a build failure.
See: https://github.com/microsoft/Detours/pull/112/checks?check_run_id=1074164464

@bgianfo
Copy link
Member

@bgianfo bgianfo commented Sep 4, 2020

Did you mean to use IMAGE_THUNK_DATA instead of IMAGE_THUNK_DATAXX? I can't find any evidence of IMAGE_THUNK_DATAXX existing in my local copy of the Windows 10 SDK for instance.

@lironzua lironzua force-pushed the lironzua:patch-4 branch from 65dd6c1 to 463a146 Sep 4, 2020
src/creatwth.cpp Outdated Show resolved Hide resolved
@lironzua lironzua force-pushed the lironzua:patch-4 branch 5 times, most recently from f01b7ab to 6195b56 Sep 4, 2020
src/uimports.cpp Outdated Show resolved Hide resolved
@lironzua lironzua force-pushed the lironzua:patch-4 branch from 6195b56 to 13543db Sep 4, 2020
@bgianfo
bgianfo approved these changes Sep 5, 2020
@bgianfo bgianfo merged commit 9fec986 into microsoft:master Sep 5, 2020
6 checks passed
6 checks passed
build (x86)
Details
build (x64)
Details
build (x64_arm)
Details
build (x64_arm64)
Details
CodeQL No new alerts
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.