Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
# - internal/reftest is a cgo differential harness (build tag refc); it is held
# to the idiomatic standard but exempted from a few cgo-hostile checks.
# - opus/, oggopus/, cmd/ are idiomatic Go at full strength.
# - modernize stays disabled: it panics on Go 1.26 + atomic types in
# golangci-lint v2 (known upstream bug), same as go-flac.
# - modernize runs on the idiomatic packages; inside the transliteration
# packages it joins the relaxed style linters, because its rangeint and
# loop rewrites fight diffability against the C. It stayed off until
# golangci-lint v2.13.2 on Go 1.27 fixed the v2 panic on atomic types
# (previously shared with go-flac).
version: "2"
output:
sort-order:
Expand All @@ -38,6 +41,7 @@ linters:
- iface
- ineffassign
- misspell
- modernize
- nilerr
- nilnil
- predeclared
Expand Down Expand Up @@ -95,6 +99,7 @@ linters:
- gocyclo
- goconst
- misspell
- modernize
- prealloc
- predeclared
- revive
Expand All @@ -110,10 +115,13 @@ linters:
linters:
- staticcheck
text: "(ST1003|ST1016|QF1|S1021)"
# cgo differential harness: cgo pointer patterns and C-mirroring names.
# cgo differential harness: cgo pointer patterns, C-mirroring names, and
# C-mirroring loops (modernize's rangeint rewrites fight diffability with
# the libopus reference the harness is checked against).
- path: internal/reftest/
linters:
- gocritic
- modernize
- revive
# Test files: table-driven tests are naturally long and repetitive.
- path: _test\.go
Expand Down
2 changes: 1 addition & 1 deletion internal/packet/toc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestParseTOCConfigTable(t *testing.T) {
// Sweep the stereo flag and all four frame-count codes for each config
// to prove those low bits do not perturb the config decoding.
for _, stereo := range []bool{false, true} {
for code := uint8(0); code < 4; code++ {
for code := range uint8(4) {
b := tc.config<<3 | code
if stereo {
b |= 0x4
Expand Down
5 changes: 1 addition & 4 deletions oggopus/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,7 @@ func (d *Decoder) fill() (done bool, err error) {
}
// End-trim so the total delivered equals finalGranule - preSkip.
if d.limit >= 0 && d.delivered+int64(perChan) > d.limit {
keep := int(d.limit - d.delivered)
if keep < 0 {
keep = 0
}
keep := max(int(d.limit-d.delivered), 0)
samples = samples[:keep*d.info.Channels]
Comment on lines 202 to 204

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good eye, but it's safe as written and this change is behavior-preserving: the old keep := int(d.limit-d.delivered); if keep < 0 { keep = 0 } and the new keep := max(int(d.limit-d.delivered), 0) do the int64 to int conversion in exactly the same place, so nothing about the conversion changed here.

At this point d.limit - d.delivered is bounded to [0, perChan). The clamp only runs under d.limit >= 0 && d.delivered+perChan > d.limit, and delivered never exceeds limit (once the end-trim fires, delivered is pinned to limit). perChan is a single Opus frame (<= 5760 samples per channel), so the value fits int32 on every platform, and the max(..., 0) floors the late-discovered-limit case to 0. The only way to feed a large-magnitude int64 into that conversion is a limit discovered late after roughly 2 billion over-delivered samples on a 32-bit build, which is pre-existing and orthogonal to enabling a linter.

I'll track hardening the clamp to int64 (and capping to perChan) as a separate follow-up rather than fold an untrusted-input logic change into this PR.

perChan = keep
}
Expand Down
5 changes: 1 addition & 4 deletions oggopus/family1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,7 @@ func genInterleavedPCM(n, channels int) []int16 {
// firstDiffByte returns the index of the first differing byte, or the shorter
// length when one is a prefix of the other.
func firstDiffByte(a, b []byte) int {
n := len(a)
if len(b) < n {
n = len(b)
}
n := min(len(b), len(a))
for i := range n {
if a[i] != b[i] {
return i
Expand Down
5 changes: 1 addition & 4 deletions oggopus/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,11 @@ func (cw *containerWriter) writePacket(pkt []byte, samples48k int) error {
// end-of-stream flag. close is idempotent.
func (cw *containerWriter) close(sourceSamples int64) error {
if cw.hasHeld {
finalGranule := cw.preSkip + sourceSamples
// Guard monotonicity: the final granule must not regress below the
// previous page's cumulative count. This never triggers for consistent
// inputs; it defends against a caller passing a sourceSamples smaller
// than the already-committed audio.
if finalGranule < cw.granule {
finalGranule = cw.granule
}
finalGranule := max(cw.preSkip+sourceSamples, cw.granule)
if err := cw.sw.writeAudioPacket(cw.heldPacket, finalGranule); err != nil {
return err
}
Expand Down
Loading