Merge Noble -> Resolute#645
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.go`:
- Around line 6401-6418: The GPIO_V2_LINE_SET_CONFIG_IOCTL constant is missing
from the GPIO v2 ioctl constants block. Add this constant to the const block
that contains GPIO_V2_GET_LINEINFO_IOCTL, GPIO_V2_GET_LINE_IOCTL,
GPIO_V2_LINE_GET_VALUES_IOCTL, GPIO_V2_LINE_SET_VALUES_IOCTL,
GPIO_V2_GET_LINEINFO_WATCH_IOCTL, and GPIO_GET_LINEINFO_UNWATCH_IOCTL. The
constant should be included in this first const block along with the other ioctl
definitions to complete the GPIO v2 API surface.
In `@acceptance-tests/vendor/golang.org/x/sys/windows/setupapi_windows.go`:
- Around line 1174-1192: Add an alignment-safety note or check around bufToUTF16
and utf16ToBuf in setupapi_windows.go: these unsafe slice reinterprets via the
local struct/unsafe.Pointer need explicit alignment assumptions. Update the
comments on bufToUTF16 and utf16ToBuf to state that callers must pass properly
aligned buffers, or add a small runtime alignment guard before constructing the
[]uint16/[]byte view.
- Around line 1385-1415: Update CM_Get_Device_Interface_List so the no-results
case returns an empty slice and no error instead of
ERROR_NO_SUCH_DEVICE_INTERFACE. In the final loop over the buffer, keep the
existing parsing logic, but change the nil interfaces branch at the end of
CM_Get_Device_Interface_List to return an empty []string with nil error,
matching the Windows API behavior when no interfaces are found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8791706c-f258-4489-806f-ae13b886f57e
⛔ Files ignored due to path filters (10)
Gemfile.lockis excluded by!**/*.lockacceptance-tests/go.sumis excluded by!**/*.sumvendor/package/json-2.19.8.gemis excluded by!vendor/**vendor/package/json-2.19.9.gemis excluded by!vendor/**vendor/package/parallel-1.28.0.gemis excluded by!vendor/**vendor/package/parallel-2.1.0.gemis excluded by!vendor/**vendor/package/rubocop-1.84.2.gemis excluded by!vendor/**vendor/package/rubocop-1.87.0.gemis excluded by!vendor/**vendor/package/standard-1.54.0.gemis excluded by!vendor/**vendor/package/standard-1.55.0.gemis excluded by!vendor/**
📒 Files selected for processing (66)
acceptance-tests/go.modacceptance-tests/vendor/github.com/cloudfoundry/bosh-utils/system/cmd_runner_interface.goacceptance-tests/vendor/github.com/cloudfoundry/bosh-utils/system/exec_cmd_runner.goacceptance-tests/vendor/github.com/cloudfoundry/bosh-utils/system/process_priority_unix.goacceptance-tests/vendor/github.com/cloudfoundry/bosh-utils/system/process_priority_windows.goacceptance-tests/vendor/golang.org/x/net/html/entity.goacceptance-tests/vendor/golang.org/x/net/html/escape.goacceptance-tests/vendor/golang.org/x/net/html/foreign.goacceptance-tests/vendor/golang.org/x/net/html/parse.goacceptance-tests/vendor/golang.org/x/net/html/token.goacceptance-tests/vendor/golang.org/x/sync/errgroup/errgroup.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_386.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_amd64.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_arm.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_arm64.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_loong64.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_mips.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_mips64.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_mips64le.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_mipsle.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_ppc.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_ppc64.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_ppc64le.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_riscv64.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_s390x.goacceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux_sparc64.goacceptance-tests/vendor/golang.org/x/sys/windows/aliases.goacceptance-tests/vendor/golang.org/x/sys/windows/dll_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/env_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/eventlog.goacceptance-tests/vendor/golang.org/x/sys/windows/exec_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/memory_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/mkerrors.bashacceptance-tests/vendor/golang.org/x/sys/windows/mkknownfolderids.bashacceptance-tests/vendor/golang.org/x/sys/windows/mksyscall.goacceptance-tests/vendor/golang.org/x/sys/windows/race.goacceptance-tests/vendor/golang.org/x/sys/windows/race0.goacceptance-tests/vendor/golang.org/x/sys/windows/security_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/service.goacceptance-tests/vendor/golang.org/x/sys/windows/setupapi_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/str.goacceptance-tests/vendor/golang.org/x/sys/windows/syscall.goacceptance-tests/vendor/golang.org/x/sys/windows/syscall_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/types_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/types_windows_386.goacceptance-tests/vendor/golang.org/x/sys/windows/types_windows_amd64.goacceptance-tests/vendor/golang.org/x/sys/windows/types_windows_arm.goacceptance-tests/vendor/golang.org/x/sys/windows/types_windows_arm64.goacceptance-tests/vendor/golang.org/x/sys/windows/zerrors_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/zknownfolderids_windows.goacceptance-tests/vendor/golang.org/x/sys/windows/zsyscall_windows.goacceptance-tests/vendor/modules.txtimage-metalinks/ubuntu-noble/ubuntu-noble.meta4stemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-azure-storage.sha256sumstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-azure-storage.urlstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-azure-storage.versionstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-gcs.sha256sumstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-gcs.urlstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-gcs.versionstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-s3.sha256sumstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-s3.urlstemcell_builder/stages/blobstore_clis/assets/bosh-blobstore-s3.versionstemcell_builder/stages/bosh_aws_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_go_agent/assets/bosh-agent-versionstemcell_builder/stages/bosh_go_agent/assets/metalink.meta4
| const ( | ||
| GPIO_V2_GET_LINEINFO_IOCTL = 0xc100b405 | ||
| GPIO_V2_GET_LINE_IOCTL = 0xc250b407 | ||
| GPIO_V2_LINE_GET_VALUES_IOCTL = 0xc010b40e | ||
| GPIO_V2_LINE_SET_VALUES_IOCTL = 0xc010b40f | ||
| GPIO_V2_GET_LINEINFO_WATCH_IOCTL = 0xc100b406 | ||
| GPIO_GET_LINEINFO_UNWATCH_IOCTL = 0xc004b40c | ||
| ) | ||
| const ( | ||
| GPIO_V2_LINE_ATTR_ID_FLAGS = 0x1 | ||
| GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES = 0x2 | ||
| GPIO_V2_LINE_ATTR_ID_DEBOUNCE = 0x3 | ||
| GPIO_V2_LINE_CHANGED_REQUESTED = 0x1 | ||
| GPIO_V2_LINE_CHANGED_RELEASED = 0x2 | ||
| GPIO_V2_LINE_CHANGED_CONFIG = 0x3 | ||
| GPIO_V2_LINE_EVENT_RISING_EDGE = 0x1 | ||
| GPIO_V2_LINE_EVENT_FALLING_EDGE = 0x2 | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'GPIO_V2_LINE_SET_CONFIG_IOCTL' acceptance-tests/vendor/golang.org/x/sys/unixRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 66
🏁 Script executed:
# Check the actual file content at the specified lines
cat -n acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.go | sed -n '6401,6420p'Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 890
🏁 Script executed:
# Search for all GPIO_V2 constants in the file to see what was added
rg -n 'GPIO_V2' acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.goRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 720
🏁 Script executed:
# Check the upstream golang.org/x/sys repository or documentation for this constant
# First, let's see if there's any reference to the actual ioctl value
rg -n '0xc0[0-9a-f]{6}' acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.go | grep -i gpioRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 240
🌐 Web query:
GPIO_V2_LINE_SET_CONFIG_IOCTL golang.org/x/sys unix ioctl value
💡 Result:
The GPIO_V2_LINE_SET_CONFIG_IOCTL is a Linux kernel IOCTL used to reconfigure GPIO lines within an existing line request [1][2]. Its numeric value is defined in the Linux kernel UAPI headers as _IOWR(0xB4, 0x0D, struct gpio_v2_line_config) [3][4]. Regarding golang.org/x/sys/unix: This specific IOCTL is not currently exported as a constant in the standard golang.org/x/sys/unix package [5]. When interacting with the Linux GPIO character device in Go, developers typically define the IOCTL value manually or use a wrapper library that implements the necessary ioctl syscalls [6][7][8]. To compute or define the value in Go, you can mirror the kernel macro definition. Since _IOWR is typically defined as: #define _IOWR(type, nr, size) _IOC(_IOC_READ|_IOC_WRITE, (type), (nr), (_IOC_TYPECHECK(size))) The value for GPIO_V2_LINE_SET_CONFIG_IOCTL can be constructed using the values: - Type: 0xB4 - Number: 0x0D - Size: sizeof(struct gpio_v2_line_config) Most Go developers working with GPIO character devices opt to use existing, well-maintained libraries like warthog618/go-gpiocdev [6][7], which provide native Go interfaces for these operations, abstracting away the manual invocation of ioctl and the associated unsafe pointer handling [7]. If you must call it manually via golang.org/x/sys/unix, you would use the unix.IoctlSetPointerInt or a similar function, passing the file descriptor of the requested line and a pointer to the populated gpio_v2_line_config structure [1][5].
Citations:
- 1: https://docs.kernel.org/userspace-api/gpio/gpio-v2-line-set-config-ioctl.html
- 2: https://docs.kernel.org/next/userspace-api/gpio/chardev.html
- 3: https://github.com/torvalds/linux/blob/master/include/uapi/linux/gpio.h
- 4: https://sbexr.rabexc.org/latest/sources/27/24cd685aa26f8b.html
- 5: https://go.dev/src/cmd/vendor/golang.org/x/sys/unix/ioctl_linux.go
- 6: https://context7.com/warthog618/go-gpiocdev/llms.txt
- 7: https://github.com/warthog618/go-gpiocdev
- 8: https://pkg.go.dev/zappem.net/pub/io/gpio
🏁 Script executed:
# Search for gpio_v2_line_config in the file
rg -n 'gpio_v2_line_config' acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.go | head -20Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 66
🏁 Script executed:
# Get more context around the GPIO structures
cat -n acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.go | sed -n '6420,6500p'Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 1609
Add the missing GPIO v2 reconfiguration ioctl constant.
GPIO_V2_LINE_SET_CONFIG_IOCTL is part of the GPIO v2 API for updating a previously requested line's configuration. The GPIOV2LineConfig struct has been added, but the corresponding ioctl constant is missing, leaving the new config surface incomplete. Add it to the ioctl block at lines 6402–6407.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@acceptance-tests/vendor/golang.org/x/sys/unix/ztypes_linux.go` around lines
6401 - 6418, The GPIO_V2_LINE_SET_CONFIG_IOCTL constant is missing from the GPIO
v2 ioctl constants block. Add this constant to the const block that contains
GPIO_V2_GET_LINEINFO_IOCTL, GPIO_V2_GET_LINE_IOCTL,
GPIO_V2_LINE_GET_VALUES_IOCTL, GPIO_V2_LINE_SET_VALUES_IOCTL,
GPIO_V2_GET_LINEINFO_WATCH_IOCTL, and GPIO_GET_LINEINFO_UNWATCH_IOCTL. The
constant should be included in this first const block along with the other ioctl
definitions to complete the GPIO v2 API surface.
| // bufToUTF16 function reinterprets []byte buffer as []uint16 | ||
| func bufToUTF16(buf []byte) []uint16 { | ||
| sl := struct { | ||
| addr *uint16 | ||
| len int | ||
| cap int | ||
| }{(*uint16)(unsafe.Pointer(&buf[0])), len(buf) / 2, cap(buf) / 2} | ||
| return *(*[]uint16)(unsafe.Pointer(&sl)) | ||
| } | ||
|
|
||
| // utf16ToBuf function reinterprets []uint16 as []byte | ||
| func utf16ToBuf(buf []uint16) []byte { | ||
| sl := struct { | ||
| addr *byte | ||
| len int | ||
| cap int | ||
| }{(*byte)(unsafe.Pointer(&buf[0])), len(buf) * 2, cap(buf) * 2} | ||
| return *(*[]byte)(unsafe.Pointer(&sl)) | ||
| } |
There was a problem hiding this comment.
Unsafe slice reinterpretation in bufToUTF16() and utf16ToBuf() lacks alignment safety annotation.
Lines 1175–1181 (bufToUTF16) and 1185–1191 (utf16ToBuf) reinterpret byte/uint16 slices via unsafe struct manipulation. While the technique is correct for little-endian systems, the caller must ensure the input buffer is correctly aligned. If a misaligned pointer is passed (e.g., from &buf[1]), undefined behavior may result. Consider adding a comment or a runtime check for alignment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@acceptance-tests/vendor/golang.org/x/sys/windows/setupapi_windows.go` around
lines 1174 - 1192, Add an alignment-safety note or check around bufToUTF16 and
utf16ToBuf in setupapi_windows.go: these unsafe slice reinterprets via the local
struct/unsafe.Pointer need explicit alignment assumptions. Update the comments
on bufToUTF16 and utf16ToBuf to state that callers must pass properly aligned
buffers, or add a small runtime alignment guard before constructing the
[]uint16/[]byte view.
| func CM_Get_Device_Interface_List(deviceID string, interfaceClass *GUID, flags uint32) ([]string, error) { | ||
| deviceID16, err := UTF16PtrFromString(deviceID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| var buf []uint16 | ||
| var buflen uint32 | ||
| for { | ||
| if ret := cm_Get_Device_Interface_List_Size(&buflen, interfaceClass, deviceID16, flags); ret != CR_SUCCESS { | ||
| return nil, ret | ||
| } | ||
| buf = make([]uint16, buflen) | ||
| if ret := cm_Get_Device_Interface_List(interfaceClass, deviceID16, &buf[0], buflen, flags); ret == CR_SUCCESS { | ||
| break | ||
| } else if ret != CR_BUFFER_SMALL { | ||
| return nil, ret | ||
| } | ||
| } | ||
| var interfaces []string | ||
| for i := 0; i < len(buf); { | ||
| j := i + wcslen(buf[i:]) | ||
| if i < j { | ||
| interfaces = append(interfaces, UTF16ToString(buf[i:j])) | ||
| } | ||
| i = j + 1 | ||
| } | ||
| if interfaces == nil { | ||
| return nil, ERROR_NO_SUCH_DEVICE_INTERFACE | ||
| } | ||
| return interfaces, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
When CM_Get_Device_Interface_List succeeds but the device has no interfaces, should the function return an empty list or an error?
💡 Result:
When CM_Get_Device_Interface_List succeeds but no interfaces are found, the function returns CR_SUCCESS and provides an empty list [1][2]. The function returns the list of device interfaces as a double-null-terminated string (a multi-string, or PZZSTR/PZZWSTR) [3]. When no interfaces match the specified criteria, the buffer will contain only the terminating null character of the multi-string (i.e., an empty string), effectively representing an empty list [3]. Because the function populates the buffer with this multi-string format, callers should always check for a return value of CR_SUCCESS and then process the buffer accordingly, expecting it to be empty if no devices are found [1][2]. It is important to note that the companion function, CM_Get_Device_Interface_List_Size, will return the necessary size to hold this empty, null-terminated result (which is typically a single null character) if no interfaces are present [4].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/api/cfgmgr32/nf-cfgmgr32-cm_get_device_interface_listw
- 2: https://learn.microsoft.com/en-us/windows/win32/api/cfgmgr32/nf-cfgmgr32-cm_get_device_interface_lista
- 3: https://stackoverflow.com/questions/71676830/i-can-see-only-one-usb-string-when-i-try-to-print-all-the-usb-devices-winapi
- 4: https://learn.microsoft.com/en-us/windows/win32/api/cfgmgr32/nf-cfgmgr32-cm_get_device_interface_list_sizea
CM_Get_Device_Interface_List() incorrectly returns an error when no interfaces are found, contradicting the Windows API specification.
The function returns ERROR_NO_SUCH_DEVICE_INTERFACE at lines 1411–1413 when interfaces is nil. However, according to the Windows API documentation, CM_Get_Device_Interface_List should return CR_SUCCESS with an empty list when the device has no matching interfaces. The buffer in this case contains only a terminating null character (the multi-string format), which results in zero appends and a nil slice. The function should instead return an empty slice with no error: change the nil check to return []string{}, nil.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@acceptance-tests/vendor/golang.org/x/sys/windows/setupapi_windows.go` around
lines 1385 - 1415, Update CM_Get_Device_Interface_List so the no-results case
returns an empty slice and no error instead of ERROR_NO_SUCH_DEVICE_INTERFACE.
In the final loop over the buffer, keep the existing parsing logic, but change
the nil interfaces branch at the end of CM_Get_Device_Interface_List to return
an empty []string with nil error, matching the Windows API behavior when no
interfaces are found.
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
4 participants
No description provided.