-
Notifications
You must be signed in to change notification settings - Fork 56
add san to log manager #1379
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 san to log manager #1379
Conversation
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.
Pull Request Overview
This PR adds sanitizer support to the LogManager by integrating sanitizer functionality alongside the existing signals capability. The implementation follows the same pattern as the existing signals integration.
- Adds sanitizer header inclusion and conditional compilation support
- Implements native JNI methods for registering/unregistering sanitizer on both default and instance LogManagers
- Exposes sanitizer registration methods in the Java interface and implementation classes
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
lib/jni/LogManager_jni.cpp | Adds sanitizer header includes and implements four new JNI methods for sanitizer registration/unregistration |
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java | Implements sanitizer registration methods in the LogManagerProvider implementation |
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManager.java | Adds static methods for sanitizer registration on the default LogManager |
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/ILogManager.java | Adds sanitizer registration method signatures to the interface |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Tested in Android for Teams application
@ThomsonTan - Probably we need force merge as CI failure is unrelated, while we are looking into CI failure? |
Add JNI support for san
fix san jni with improper use of c_str
Added san support to logmanager