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.

Sign up for GitHub

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

Jump to bottom

[Bug]: Advanced Logger libraries rely on ASSERTs #567

Closed
1 task done
VivianNK opened this issue Aug 28, 2024 · 2 comments
Closed
1 task done

[Bug]: Advanced Logger libraries rely on ASSERTs #567

VivianNK opened this issue Aug 28, 2024 · 2 comments
Assignees
@VivianNK
Labels
state:needs-triage Needs to triaged to determine next steps state:stale Has not been updated in a long time type:bug Something isn't working urgency:low Little to no impact

Comments

@VivianNK
Copy link
Contributor

Is there an existing issue for this?

  • I have searched existing issues

Current Behavior

Discovered while unit testing.
Advanced Logger Libraries rely on ASSERTs to handle errors.
DXE library was fixed to not rely on asserts, and its unit test took this into account.
There is a PR up to fix this in the PeiLib (#565)
Creating a bug to check/fix remaining AdvancedLogger libraries relying on ASSERTs for code flow.

Expected Behavior

In RELEASE builds, ASSERTs are not triggered, and the code should handle any ASSERT-ed conditions gracefully. In the case of Advanced Logger, if there is a signature mismatch for example, we should return from the function and not continue trying to write.

Steps To Reproduce

  1. Create a unit test and have an advanced logger signature mismatch.

Build Environment

- OS(s): All
- Tool Chain(s): All
- Targets Impacted: RELEASE

Version Information

Tag: v2024050000.0.0
Top of tree

Urgency

Low

Are you going to fix this?

I will fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response
@VivianNK VivianNK added type:bug Something isn't working state:needs-triage Needs to triaged to determine next steps labels Aug 28, 2024
@github-actions github-actions bot added urgency:low Little to no impact labels Aug 28, 2024
@VivianNK VivianNK self-assigned this Aug 28, 2024
@VivianNK VivianNK mentioned this issue Aug 28, 2024
Merged
5 tasks
VivianNK added a commit that referenced this issue Aug 28, 2024
@VivianNK
AdvLoggerPkg: Exit from write call if signature mismatch (#565)
bde9c52 
## Description

Current behavior allows the PPI to be used if there is a signature or
version mismatch. This fix returns from the function to exit gracefully
and prevent memory corruption.

Created a bug to track this behavior elsewhere in Advanced Logger
Library: #567

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Local CI and unit test.

## Integration Instructions

N/A
@github-actions GitHub Actions
Copy link

This issue has been automatically marked as stale because it has not had activity in 45 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the state:stale Has not been updated in a long time label Oct 12, 2024
@github-actions GitHub Actions
Copy link

This issue has been automatically been closed because it did not have any activity in 45 days and no follow up within 7 days after being marked stale. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@VivianNK VivianNK

Labels
state:needs-triage Needs to triaged to determine next steps state:stale Has not been updated in a long time type:bug Something isn't working urgency:low Little to no impact
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@VivianNK