From 8e33a63ef885dee80e98be63e83a2ab1d49f2138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joa=CC=83o=20Amaro=20Lagedo?= Date: Sun, 24 May 2026 03:10:04 -0300 Subject: [PATCH 1/2] Fix serial console attachment error check newVZFileHandleSerialPortAttachment checked the error out-parameter pointer (`if (error != nil)`), which is always non-nil since callers pass the address of their error variable. The function therefore returned nil before building the attachment, silently disabling the serial console. Check the duplicated file handle instead, matching newVZFileHandleNetworkDeviceAttachment. Add a regression test asserting the constructor returns a non-NULL attachment. Co-Authored-By: Claude Opus 4.7 (1M context) --- serial_console_test.go | 27 +++++++++++++++++++++++++++ virtualization_11.m | 7 +++++-- 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 serial_console_test.go diff --git a/serial_console_test.go b/serial_console_test.go new file mode 100644 index 00000000..9ad87673 --- /dev/null +++ b/serial_console_test.go @@ -0,0 +1,27 @@ +package vz_test + +import ( + "errors" + "os" + "testing" + + "github.com/Code-Hex/vz/v3" + "github.com/Code-Hex/vz/v3/internal/objc" +) + +// TestNewFileHandleSerialPortAttachment guards against a regression where the +// constructor checked the error out-parameter pointer (always non-nil) instead +// of the duplicated file handle, causing it to return a non-nil wrapper around a +// NULL Objective-C object with a nil error. The serial console was silently lost. +func TestNewFileHandleSerialPortAttachment(t *testing.T) { + attachment, err := vz.NewFileHandleSerialPortAttachment(os.Stdin, os.Stderr) + if errors.Is(err, vz.ErrUnsupportedOSVersion) { + t.Skipf("not supported on this macOS version: %v", err) + } + if err != nil { + t.Fatal(err) + } + if objc.Ptr(attachment) == nil { + t.Fatal("attachment wraps a NULL pointer: constructor reported success but built nothing") + } +} diff --git a/virtualization_11.m b/virtualization_11.m index 28060e0f..cc6030b3 100644 --- a/virtualization_11.m +++ b/virtualization_11.m @@ -453,13 +453,16 @@ void setStorageDevicesVZVirtualMachineConfiguration(void *config, if (@available(macOS 11, *)) { VZFileHandleSerialPortAttachment *ret; @autoreleasepool { + // Check the returned handle, not `error`: `error` is the void** out-param (always + // non-nil), so `if (error != nil)` is always true. newFileHandleDupFd returns nil + // and sets *error only on failure. Matches newVZFileHandleNetworkDeviceAttachment. NSFileHandle *fileHandleForReading = newFileHandleDupFd(readFileDescriptor, error); - if (error != nil) { + if (fileHandleForReading == nil) { return nil; } NSFileHandle *fileHandleForWriting = newFileHandleDupFd(writeFileDescriptor, error); - if (error != nil) { + if (fileHandleForWriting == nil) { return nil; } ret = [[VZFileHandleSerialPortAttachment alloc] From d77a0dde6db4b137815c44d44b83ee93239f5c28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joa=CC=83o=20Amaro=20Lagedo?= Date: Wed, 27 May 2026 02:12:59 -0300 Subject: [PATCH 2/2] Remove explanatory comment per review feedback Co-Authored-By: Claude Opus 4.7 (1M context) --- virtualization_11.m | 3 --- 1 file changed, 3 deletions(-) diff --git a/virtualization_11.m b/virtualization_11.m index cc6030b3..b3abc137 100644 --- a/virtualization_11.m +++ b/virtualization_11.m @@ -453,9 +453,6 @@ void setStorageDevicesVZVirtualMachineConfiguration(void *config, if (@available(macOS 11, *)) { VZFileHandleSerialPortAttachment *ret; @autoreleasepool { - // Check the returned handle, not `error`: `error` is the void** out-param (always - // non-nil), so `if (error != nil)` is always true. newFileHandleDupFd returns nil - // and sets *error only on failure. Matches newVZFileHandleNetworkDeviceAttachment. NSFileHandle *fileHandleForReading = newFileHandleDupFd(readFileDescriptor, error); if (fileHandleForReading == nil) { return nil;