Skip to content

Add additional host based unit tests for MediaClear functions in the Nvme Media Sanitize Unit tests + fix a few more errors in the unit test#12426

Open
spbrogan wants to merge 4 commits intotianocore:masterfrom
spbrogan:improve_nvme_sanitize_unit_test
Open

Add additional host based unit tests for MediaClear functions in the Nvme Media Sanitize Unit tests + fix a few more errors in the unit test#12426
spbrogan wants to merge 4 commits intotianocore:masterfrom
spbrogan:improve_nvme_sanitize_unit_test

Conversation

@spbrogan
Copy link
Copy Markdown
Member

@spbrogan spbrogan commented Apr 9, 2026

Description

This adds test cases for two bugs found in the NVME driver.

#12396
#12397

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

These are tests. Running CI will run the host based tests.

Integration Instructions

N/A

@github-actions github-actions bot added the impact:testing This contribution includes tests such as unit and/or integration tests. label Apr 9, 2026
@spbrogan spbrogan force-pushed the improve_nvme_sanitize_unit_test branch 2 times, most recently from 8a5a1eb to 25cabb5 Compare April 9, 2026 22:54
@spbrogan spbrogan changed the title Add additional host based unit tests for MediaClear functions in the Nvme Media Sanitize Unit tests Add additional host based unit tests for MediaClear functions in the Nvme Media Sanitize Unit tests + fix a few more errors in the unit test Apr 9, 2026
spbrogan added 3 commits April 9, 2026 16:20
Add tests for MediaClear functions in the Nvme Media Sanitize Unit tests

Signed-off-by: Sean Brogan <sebrogan@microsoft.com>
NamespaceData was allocated, zeroed, and never modifed.
Then it was copied over another buffer that was already zero.
Then it was leaked.

Remove local variable NamespaceData and the associated code.

Signed-off-by: Sean Brogan <sebrogan@microsoft.com>
LastBlock field is inclusive.

To lower confusion it should be initialized to the intended
value which was to create metadata for an 8mb disk.

Signed-off-by: Sean Brogan <sebrogan@microsoft.com>
@spbrogan spbrogan force-pushed the improve_nvme_sanitize_unit_test branch from 25cabb5 to 764db5e Compare April 9, 2026 23:21
Signed-off-by: Sean Brogan <sebrogan@microsoft.com>
Device->Media.BlockSize = (UINT32)(1 << 9); // 512 byte sector size

Device->Media.LastBlock = 0x4000; // NamespaceData=>Nsze
Device->Media.LastBlock = 0x3FFF;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming the original comment made some sense, you could just append - 1 to the comment text and keep it?

@spbrogan
Copy link
Copy Markdown
Member Author

The test failure is expected. The unit test is catching the issues fixed by pr #12424 pr #12423

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:testing This contribution includes tests such as unit and/or integration tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants