Skip to content

Minor follow-up initrd feedback items#14186

Merged
benhillis merged 1 commit into
masterfrom
user/benhill/initrd_cleanup
Feb 9, 2026
Merged

Minor follow-up initrd feedback items#14186
benhillis merged 1 commit into
masterfrom
user/benhill/initrd_cleanup

Conversation

@benhillis

Copy link
Copy Markdown
Member

Some minor feedback items that came up during the initrd change that I wanted to wait until after merging.

Comment thread .gitignore
@@ -10,7 +10,6 @@ tmp
*.user

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

unrelated but I keep getting annoyed that the test-storage directory isn't in the gitignore in the main branch.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Follow-up adjustments around initrd/initramfs (CPIO “newc”) generation and installation behavior, plus a small cleanup to repo ignore patterns.

Changes:

  • Update the CPIO initrd writer to treat only the "TRAILER!!!" entry as the special zeroed header, and to compute archive padding using the full file size (QuadPart).
  • Adjust the Windows SimpleTests CPIO validation to read exactly the 110-byte newc header while keeping a null-terminated buffer for parsing.
  • Update MSI installer sequencing comment for initrd generation and tweak .gitignore entries.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
test/windows/SimpleTests.cpp Reads/parses the fixed-size 110-byte CPIO newc header with explicit null termination.
src/windows/common/filesystem.cpp Fixes CPIO header field selection for trailer vs. empty files; fixes padding calculation to use 64-bit current position.
msipackage/package.wix.in Clarifies when CreateInitrd runs by updating the inline comment.
.gitignore Adjusts ignored patterns (removes *.targets and build/, adds a few specific ignores).
Comment on lines +330 to +331
THROW_IF_WIN32_BOOL_FALSE(ReadFile(cpioHandle.get(), header, 110, &bytesRead, nullptr));
VERIFY_ARE_EQUAL(bytesRead, 110u);

Copilot AI Feb 9, 2026

Copy link

Choose a reason for hiding this comment

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

The header size is hard-coded as 110 in both the ReadFile length and the VERIFY_ARE_EQUAL check. Since the buffer size was just changed to 111 to include a null terminator, consider using a single named constant (or sizeof(header) - 1) to keep the read length/check consistent and avoid future off-by-one drift.

Suggested change
THROW_IF_WIN32_BOOL_FALSE(ReadFile(cpioHandle.get(), header, 110, &bytesRead, nullptr));
VERIFY_ARE_EQUAL(bytesRead, 110u);
const DWORD headerBytesToRead = static_cast<DWORD>(sizeof(header) - 1);
THROW_IF_WIN32_BOOL_FALSE(ReadFile(cpioHandle.get(), header, headerBytesToRead, &bytesRead, nullptr));
VERIFY_ARE_EQUAL(bytesRead, headerBytesToRead);
Copilot uses AI. Check for mistakes.
Comment on lines 1168 to +1172
0,
(fileSize > 0) ? 0100755 : 0,
isTrailer ? 0 : 0100755,
0,
0,
(fileSize > 0) ? 1 : 0,
isTrailer ? 0 : 1,

Copilot AI Feb 9, 2026

Copy link

Choose a reason for hiding this comment

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

This changes the CPIO header fields so that empty files (fileSize==0) still get a regular-file mode/nlink, and only the trailer gets zeros. The existing CreateCpioInitrd test parses filesize/namesize but doesn’t assert mode/nlink, so it wouldn’t catch regressions in this logic. Consider extending the test to parse and validate mode/nlink at least for sourceSize=0 (and ideally also validate the trailer entry).

Copilot uses AI. Check for mistakes.
@benhillis benhillis merged commit ae39345 into master Feb 9, 2026
12 checks passed
benhillis added a commit that referenced this pull request Feb 13, 2026
* Move from shipping the initrd to generating it during install. (#14119)

* Move from shipping the initrd to generating during package install.

* pr feedback

* working

* adjust custom action conditions

* update initrd test to cover more cases

* Update msipackage/package.wix.in

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* use stack buffer

* move initrd helper to filesystem.cpp and add unit test

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Minor follow-up initrd feedback items (#14186)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* Add Clang requirement to vswhere usage (#14187)

* Enable detection cmake script for Visual Studio 2026 and pre-release versions of Visual Studio (#14160)

* Updating to support VS2026 and insiders builds

* Updated max ver (exclusive) to 19.0

* Fix command for vswhere to include prerelease

---------

Co-authored-by: Ben Hillis <benhillis@gmail.com>

* Update DistributionInfo.json due release of Ubuntu-24.04.4 (#14202)

* Update Microsoft.WSL.DeviceHost with virtiofs and virtio networking (#14198)

improvements.

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* virtioproxy: update setting of m_networkSettings to under the lock (#14210)

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

* update HcsVirtualMachine with new VirtioNetworking behavior

---------

Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: JohnMcPMS <johnmcp@microsoft.com>
Co-authored-by: Andy Sterland <andster@microsoft.com>
Co-authored-by: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants