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

AdvLoggerPkg: Fix message handling and LineParserTestApp #634

Conversation

makubacki
Copy link
Member

@makubacki makubacki commented Feb 18, 2025

Description

Fixes #629

A case existed where the target buffer was not correct whenever the same line buffer was extended (previous message overwritten).

Debug level strings could overwrite characters if a new debug level was changed when residual characters were added to the same line buffer. To simplify handling of strings, this change makes the phases and debug message levels a fixed size. This is a breaking change but less error prone.

The LineParserTestApp needed to be updated for debug level strings, so that is done.

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?
  • Backport to release branch?

How This Was Tested

  • LineParserTestApp
  • Step through parsing with debugger and check variable values
  • Log output

Integration Instructions

  • If any tools or code depend on exact string matches, note the change in the debug message level and phase strings.

@makubacki makubacki added the type:bug Something isn't working label Feb 18, 2025
@makubacki makubacki requested review from kuqin12 and apop5 February 18, 2025 21:31
@makubacki makubacki self-assigned this Feb 18, 2025
@github-actions github-actions bot added impact:breaking-change Requires integration attention impact:non-functional Does not have a functional impact impact:testing Affects testing type:backport Backport changes in a dev branch PR to its release branch. labels Feb 18, 2025
@makubacki makubacki removed the impact:non-functional Does not have a functional impact label Feb 18, 2025
@makubacki makubacki force-pushed the fix_advlogger_phase_dbglevel_handling branch from 18b8607 to ad41c4c Compare February 18, 2025 22:17
A case existed where the target buffer was not correct whenever the
same line buffer was extended (previous message overwritten).

Debug level strings could overwrite characters if a new debug level
was changed when residual characters were added to the same line
buffer. To simplify handling of strings, this change makes the
phases and debug message levels a fixed size. This is a breaking
change but less error prone.

The LineParserTestApp needed to be updated for debug level strings,
so that is done.

Signed-off-by: Michael Kubacki <[email protected]>
@makubacki makubacki force-pushed the fix_advlogger_phase_dbglevel_handling branch from ad41c4c to 76147d2 Compare February 18, 2025 22:26
@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 0% with 31 lines in your changes missing coverage. Please review.

Project coverage is 11.59%. Comparing base (f55d046) to head (76147d2).

Files with missing lines Patch % Lines
.../AdvancedLoggerAccessLib/AdvancedLoggerAccessLib.c 0.00% 31 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           dev/202405     #634   +/-   ##
===========================================
  Coverage       11.59%   11.59%           
===========================================
  Files             132      132           
  Lines           21561    21555    -6     
  Branches         2546     2546           
===========================================
  Hits             2499     2499           
+ Misses          19028    19022    -6     
  Partials           34       34           
Flag Coverage Δ
AdvLoggerPkg 6.08% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@makubacki makubacki requested a review from cfernald February 19, 2025 15:38
@makubacki makubacki merged commit 4ac6d20 into microsoft:dev/202405 Feb 25, 2025
28 checks passed
ProjectMuBot pushed a commit that referenced this pull request Feb 25, 2025
## Description

Fixes #629 

A case existed where the target buffer was not correct whenever the same
line buffer was extended (previous message overwritten).

Debug level strings could overwrite characters if a new debug level was
changed when residual characters were added to the same line buffer. To
simplify handling of strings, this change makes the phases and debug
message levels a fixed size. This is a breaking change but less error
prone.

The LineParserTestApp needed to be updated for debug level strings, so
that is done.

- [x] Impacts functionality?
- [ ] Impacts security?
- [x] Breaking change?
- [x] Includes tests?
- [ ] Includes documentation?
- [x] Backport to release branch?

## How This Was Tested

- LineParserTestApp
- Step through parsing with debugger and check variable values
- Log output

## Integration Instructions

- If any tools or code depend on exact string matches, note the change
in the debug message level and phase strings.

Signed-off-by: Michael Kubacki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:breaking-change Requires integration attention impact:testing Affects testing type:backport Backport changes in a dev branch PR to its release branch. type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: LineParserTestApp is Broken
5 participants