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

Add tests (and refactor) API extraction and symbol generation #2597

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

v1bh475u
Copy link
Contributor

@v1bh475u v1bh475u commented Feb 7, 2025

closes #1899

Checklist

  • No CHANGELOG update needed
  • No new tests needed
  • No documentation update needed

Comment on lines +1662 to +1665
assert trim_dll_part("kernel32.CreateFileA") == "CreateFileA"
assert trim_dll_part("System.Convert::FromBase64String") == "System.Convert::FromBase64String"
assert trim_dll_part("System.Diagnostics.Debugger::IsLogging") == "System.Diagnostics.Debugger::IsLogging"
assert trim_dll_part("ws2_32.#1") == "ws2_32.#1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems inconsistent to me:

assert trim_dll_part("kernel32.CreateFileA") == "CreateFileA" only export name.
assert trim_dll_part("ws2_32.#1") == "ws2_32.#1" both DLL and export name.

would you please continue to explore how this code is used (e.g., review the callers) and consider if further refactoring needs to be done to make the code less surprising?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so we can remove the dll part for ordinal function. Reason: ordinal is meaningless without the dll part. In case of named functions, only a single library contains them and exports them but ordinal is same for all libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is used for api and count(api:<api_name>) statement building

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests (and refactor) API extraction and symbol generation
2 participants