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 + Tests: Add more tests covering the modules API, fix error handling bug that were found. #177

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

bgianfo
Copy link
Contributor

@bgianfo bgianfo commented Mar 1, 2021

This PR add more tests for the basic functionality and error handling of the Detours Module API.

  • DetourLoadImageHlp
  • DetourFindFunction
  • DetourEnumerateModules
  • DetourEnumerateExports
  • DetourEnumerateImports
  • DetourGetSizeOfPayloads
  • DetourFindPayload
  • DetourFindPayloadEx
  • DetourRestoreAfterWithEx

It also fixes some error handling issues that the tests found in the module API.

  • DetoursFindFunction wasn't gracefully handling NULL function name.
  • DetourEnumerateModules wasn't resetting GLE on success.
  • DetourEnumerateExports wasn't gracefully handling NULL export callback.
  • DetourEnumerateImports wasn't gracefully handling NULL arguments.
Microsoft Reviewers: Open in CodeFlow

- `DetoursFindFunction` wasn't gracefully handling NULL function name.

- `DetourEnumerateModules` wasn't resetting GLE on success.

- `DetourEnumerateExports` wasn't gracefully handling NULL export callback.

- `DetourEnumerateImports` wasn't gracefully handling NULL arguments.
@bgianfo bgianfo added bug Something isn't working enhancement This adds new functionallity to the product labels Mar 2, 2021
@bgianfo bgianfo force-pushed the add-more-tests branch 2 times, most recently from 9b200f5 to 3bfbe37 Compare March 2, 2021 08:33
Test basic functionality and error handling of the Detours Module API.

* DetourLoadImageHlp
* DetourFindFunction
* DetourEnumerateModules
* DetourEnumerateExports
* DetourEnumerateImports
* DetourGetSizeOfPayloads
* DetourFindPayload
* DetourFindPayloadEx
* DetourRestoreAfterWithEx
@bgianfo bgianfo added the auto-merge Used to mark a PR as mergeable when all policies pass. label Mar 2, 2021
@ghost
Copy link

ghost commented Mar 2, 2021

Hello @bgianfo!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@bgianfo bgianfo added auto-merge Used to mark a PR as mergeable when all policies pass. and removed auto-merge Used to mark a PR as mergeable when all policies pass. labels Mar 3, 2021
@bgianfo bgianfo requested a review from dtarditi March 3, 2021 10:05
Copy link
Contributor

@dtarditi dtarditi left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the tests and fixes!

@bgianfo bgianfo added the auto-merge Used to mark a PR as mergeable when all policies pass. label Mar 5, 2021
@ghost
Copy link

ghost commented Mar 5, 2021

Apologies, I am afraid I am encountering technical difficulties that might have hampered my ability to assist with merging this pull request. I will continue to try to assist if there are further changes to this pull request.

@microsoft microsoft deleted a comment Mar 5, 2021
@microsoft microsoft deleted a comment Mar 5, 2021
@bgianfo bgianfo merged commit 0a3ab89 into microsoft:master Mar 5, 2021
@bgianfo bgianfo deleted the add-more-tests branch March 5, 2021 06:26
number201724 pushed a commit to number201724/Detours that referenced this pull request Mar 5, 2021
…ing bug that were found. (microsoft#177)

* Fix: Handle more error cases in module API.

- `DetoursFindFunction` wasn't gracefully handling NULL function name.

- `DetourEnumerateModules` wasn't resetting GLE on success.

- `DetourEnumerateExports` wasn't gracefully handling NULL export callback.

- `DetourEnumerateImports` wasn't gracefully handling NULL arguments.

* Tests: Add more tests covering the modules API.

Test basic functionality and error handling of the Detours Module API.

* DetourLoadImageHlp
* DetourFindFunction
* DetourEnumerateModules
* DetourEnumerateExports
* DetourEnumerateImports
* DetourGetSizeOfPayloads
* DetourFindPayload
* DetourFindPayloadEx
* DetourRestoreAfterWithEx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Used to mark a PR as mergeable when all policies pass. bug Something isn't working enhancement This adds new functionallity to the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants