Skip to content

Add defer cleanup to NewGTU7 to prevent serial port leak#28

Closed
Copilot wants to merge 3 commits intogtu7from
copilot/sub-pr-16-7a1d46b9-cdf8-45bb-809c-0ac6874ea639
Closed

Add defer cleanup to NewGTU7 to prevent serial port leak#28
Copilot wants to merge 3 commits intogtu7from
copilot/sub-pr-16-7a1d46b9-cdf8-45bb-809c-0ac6874ea639

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 16, 2026

Addresses resource leak in NewGTU7 where a panic after successful OpenSerial would leave the port open.

Changes

  • Added defer cleanup pattern using ok flag to close serial port on panic
  • Follows established pattern from drivers/serial_linux.go:30-35
port, err := cfg.Factory.OpenSerial(cfg.Serial)
if err != nil {
    panic(err)
}

// Ensure we don't leak the port if a panic occurs after opening.
ok := false
defer func() {
    if !ok {
        if closer, canClose := port.(io.Closer); canClose {
            _ = closer.Close()
        }
    }
}()

r = port
ok = true

The type assertion to io.Closer handles both SerialPort returns and any future interface changes.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits January 16, 2026 18:05
Co-authored-by: rustyeddy <2903425+rustyeddy@users.noreply.github.com>
Co-authored-by: rustyeddy <2903425+rustyeddy@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 16, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • go.googlesource.com
    • Triggering command: /update-job-proxy /update-job-proxy 4303997/b011/ (dns block)
    • Triggering command: /update-job-proxy /update-job-proxy -o br-d8f236fa6d57 -j DROP ertum_EC-384_CA.pem .8/x64/codeql/to/usr/share/ca-certificates/mozilla/Security_Communication_ECC_RootCA1.crt test eUeXgd7p LBT77L5xt rgo/bin/git test -e 2888f65.0 AIKBV6smDMe5S/ne/usr/share/ca-certificates/mozilla/emSign_ECC_Root_CA_-_G3.crt /usr/bin/test a56daae944d3c60c/usr/sbin/iptables ication_Authorit--wait ache/go/1.24.11/-t test (dns block)
  • gopkg.in
    • Triggering command: /update-job-proxy /update-job-proxy 4303997/b011/ (dns block)
    • Triggering command: /update-job-proxy /update-job-proxy -o br-d8f236fa6d57 -j DROP ertum_EC-384_CA.pem .8/x64/codeql/to/usr/share/ca-certificates/mozilla/Security_Communication_ECC_RootCA1.crt test eUeXgd7p LBT77L5xt rgo/bin/git test -e 2888f65.0 AIKBV6smDMe5S/ne/usr/share/ca-certificates/mozilla/emSign_ECC_Root_CA_-_G3.crt /usr/bin/test a56daae944d3c60c/usr/sbin/iptables ication_Authorit--wait ache/go/1.24.11/-t test (dns block)
  • periph.io
    • Triggering command: /update-job-proxy /update-job-proxy 4303997/b011/ (dns block)
    • Triggering command: /update-job-proxy /update-job-proxy -o br-d8f236fa6d57 -j DROP ertum_EC-384_CA.pem .8/x64/codeql/to/usr/share/ca-certificates/mozilla/Security_Communication_ECC_RootCA1.crt test eUeXgd7p LBT77L5xt rgo/bin/git test -e 2888f65.0 AIKBV6smDMe5S/ne/usr/share/ca-certificates/mozilla/emSign_ECC_Root_CA_-_G3.crt /usr/bin/test a56daae944d3c60c/usr/sbin/iptables ication_Authorit--wait ache/go/1.24.11/-t test (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Address changes based on feedback from Gtu7 sensor PR review Add defer cleanup to NewGTU7 to prevent serial port leak Jan 16, 2026
Copilot AI requested a review from rustyeddy January 16, 2026 18:08
@rustyeddy rustyeddy closed this Jan 17, 2026
@rustyeddy rustyeddy deleted the copilot/sub-pr-16-7a1d46b9-cdf8-45bb-809c-0ac6874ea639 branch January 17, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants