Skip to content

Use pointer types for DialConfig fields#269

Draft
asayyah wants to merge 2 commits intomainfrom
fix/dial-config-pointer-types
Draft

Use pointer types for DialConfig fields#269
asayyah wants to merge 2 commits intomainfrom
fix/dial-config-pointer-types

Conversation

@asayyah
Copy link
Copy Markdown

@asayyah asayyah commented Feb 26, 2026

Summary

  • Change TLSConfig and DTLSConfig fields in DialConfig from value types (tls.Config, dtls.Config) to pointers (*tls.Config, *dtls.Config)
  • Removes the //nolint:govet, copylocks suppression that was needed for the value-copy pattern
  • DialURI handles nil configs by falling back to zero-value defaults, and clones before mutating ServerName to avoid side effects

Fixes #235

Test plan

  • go test -race ./... passes
  • go vet ./... clean (copylocks warning gone)
  • golangci-lint run ./... reports 0 issues
  • go build ./... compiles all packages and CLI tools

Change TLSConfig and DTLSConfig fields in DialConfig
from value types to pointers (*tls.Config, *dtls.Config).

This avoids copylocks warnings since tls.Config contains
a sync.Mutex, and lets callers pass existing configs
without dereferencing. DialURI handles nil by falling
back to a zero-value config, and clones before mutating
ServerName to avoid side effects.

Fixes #235
@asayyah asayyah marked this pull request as draft February 26, 2026 02:17
@JoTurk
Copy link
Copy Markdown
Member

JoTurk commented Feb 26, 2026

We should plan for a major stun upgrade soon, after we upgrade all of pion to turn@v5.

Test that nil TLSConfig/DTLSConfig works, and that
the caller's configs are not mutated by DialURI.
@asayyah
Copy link
Copy Markdown
Author

asayyah commented Feb 26, 2026

I'll keep this PR in draft mode until we are ready for the next upgrade

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.50%. Comparing base (31c4046) to head (ff6d191).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
+ Coverage   65.91%   66.50%   +0.59%     
==========================================
  Files          27       27              
  Lines        2077     2081       +4     
==========================================
+ Hits         1369     1384      +15     
+ Misses        697      683      -14     
- Partials       11       14       +3     
Flag Coverage Δ
go 66.50% <100.00%> (+0.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoTurk
Copy link
Copy Markdown
Member

JoTurk commented May 5, 2026

@asayyah Hello, I think we can release a new major version soon after all the strict mode work and the API refactors, do you want to fix the conflicts and get this ready to review?

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.

stun.DialConfig{} requires a tls.Config instead of *tls.Config

2 participants