From 10672f38163e47bd1138721c361babd582ab14ce Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Sun, 21 Jun 2026 15:45:56 +0900 Subject: [PATCH] all: cut bloat found in the pike audit Dead, vestigial, and over-built code with no behavior change (except the bool-vocabulary trim, a deliberate behavior tightening): - httpdl: drop the vestigial sort+copy in validSegs (work-stealing was removed in 270812d, so segments always tile [0,total) ascending now); validate order in place, which also rejects a scrambled sidecar. Inline the endOff() accessor to s.end. - cli: shrink the boolean vocabulary to true/false, which is all that's accepted. The invented yes/no/1/0/on/off spellings are gone, so e.g. --enable-dht=yes (or yes in got.conf) is now a usage error. - download: remove the dead, never-called Stat.Done() method. - progress: drop speed(), a pure alias of humanSize; call humanSize directly. - main: replace the jobsHolder mutex box with atomic.Pointer[[]job] for the forced-exit session save; the capability stays, the lock and type go. - bt: collapse isAddrInUse's X||X (missinggo.IsAddrInUse is that exact string match); drop the now-unused missinggo import (tidy -> indirect). - option: note that prealloc and falloc are identical here. --- bt/bt.go | 8 ++++---- cli/option.go | 2 +- cli/options.go | 13 +++++++------ cli/parse_test.go | 10 +++++++--- download/download.go | 3 --- go.mod | 2 +- httpdl/control.go | 12 +++++------- httpdl/httpdl.go | 2 +- httpdl/segment.go | 1 - httpdl/segment_test.go | 2 +- main.go | 41 +++++++++++++++-------------------------- progress/format.go | 6 ------ progress/format_test.go | 10 ---------- progress/progress.go | 10 +++++----- 14 files changed, 47 insertions(+), 75 deletions(-) diff --git a/bt/bt.go b/bt/bt.go index 4ee8eb4..f4199d8 100644 --- a/bt/bt.go +++ b/bt/bt.go @@ -19,7 +19,6 @@ import ( "time" alog "github.com/anacrolix/log" - missinggo "github.com/anacrolix/missinggo/v2" "github.com/anacrolix/torrent" "github.com/anacrolix/torrent/metainfo" "github.com/hanbok/got/download" @@ -172,10 +171,11 @@ func NewClient(cfg ClientConfig) (*torrent.Client, error) { return cl, nil } -// isAddrInUse reports whether err is an "address already in use" bind failure, -// matching either missinggo's helper or the raw string the library wraps. +// isAddrInUse reports whether err is an "address already in use" bind failure. +// (anacrolix/missinggo's IsAddrInUse is this exact string match, so we just do +// it directly rather than carry the import for it.) func isAddrInUse(err error) bool { - return err != nil && (missinggo.IsAddrInUse(err) || strings.Contains(err.Error(), "address already in use")) + return err != nil && strings.Contains(err.Error(), "address already in use") } // Options are the per-download settings. diff --git a/cli/option.go b/cli/option.go index 76e7d24..c6355c8 100644 --- a/cli/option.go +++ b/cli/option.go @@ -122,7 +122,7 @@ var options = []Opt{ {Long: "allow-overwrite", Kind: Bool, Default: "false", Help: "overwrite an existing file", Tag: Advanced}, {Long: "auto-file-renaming", Kind: Bool, Default: "true", Help: "rename file (.1, .2, ...) if it already exists", Tag: Advanced}, {Long: "human-readable", Kind: Bool, Default: "true", Help: "show sizes as Ki/Mi/Gi", Tag: Advanced}, - {Long: "file-allocation", Short: 'a', Kind: Enum, Default: "prealloc", Enum: []string{"none", "prealloc", "trunc", "falloc"}, Help: "how to allocate disk space", Tag: Advanced}, + {Long: "file-allocation", Short: 'a', Kind: Enum, Default: "prealloc", Enum: []string{"none", "prealloc", "trunc", "falloc"}, Help: "how to reserve disk space (prealloc and falloc are identical here)", Tag: Advanced}, {Long: "dry-run", Kind: Bool, Default: "false", Help: "check that the file is available but do not download it", Tag: Advanced}, {Long: "stop", Kind: Int, Default: "0", Min: 0, Help: "stop the program after N seconds (0 = off)", Tag: Advanced}, {Long: "disable-ipv6", Kind: Bool, Default: "false", Help: "disable IPv6 (force IPv4-only connections)", Tag: Advanced}, diff --git a/cli/options.go b/cli/options.go index 243ce10..78fc0f0 100644 --- a/cli/options.go +++ b/cli/options.go @@ -27,13 +27,14 @@ func (o *Options) IsSet(name string) bool { return o.set[name] } // Str returns the raw string value (empty if unset and no default). func (o *Options) Str(name string) string { return o.vals[name] } -// boolWords is the single accepted vocabulary for boolean options, mapping each -// recognised spelling to its truth value. The Bool reader, truthy, and -// validate() all consult it, so exactly the words that validate are honoured -// (no "accepted by the reader but rejected by validate" surprises like --x=on). +// boolWords is the accepted vocabulary for boolean options. aria2 takes only +// true/false (and rejects everything else), so we match it rather than inventing +// extra spellings. boolWord is the single consult point — the Bool reader, +// validate(), and the no-conf bootstrap all go through it — so exactly the words +// that validate are honoured. var boolWords = map[string]bool{ - "true": true, "yes": true, "1": true, "on": true, - "false": false, "no": false, "0": false, "off": false, + "true": true, + "false": false, } // boolWord reports a value's truth and whether it is a recognised boolean word. diff --git a/cli/parse_test.go b/cli/parse_test.go index 83be192..caeaea3 100644 --- a/cli/parse_test.go +++ b/cli/parse_test.go @@ -139,13 +139,17 @@ func TestValidateFloatNonNegative(t *testing.T) { func TestValidateBool(t *testing.T) { b := byLong["enable-dht"] - for _, ok := range []string{"true", "false", "yes", "no", "1", "0", "on", "off"} { + // aria2 accepts only true/false; the invented yes/no/1/0/on/off spellings + // were removed, so they must now fail validation. + for _, ok := range []string{"true", "false"} { if err := validate(b, ok); err != nil { t.Errorf("validate bool %q: %v", ok, err) } } - if err := validate(b, "flase"); err == nil { - t.Errorf("validate bool flase: want error") + for _, bad := range []string{"yes", "no", "1", "0", "on", "off", "flase"} { + if err := validate(b, bad); err == nil { + t.Errorf("validate bool %q: want error (only true/false accepted)", bad) + } } } diff --git a/download/download.go b/download/download.go index 9b008f0..8a0e6c1 100644 --- a/download/download.go +++ b/download/download.go @@ -55,9 +55,6 @@ type Stat struct { Seeders int // connected seeders (BitTorrent only) } -// Done reports whether the download has reached a terminal state. -func (s Stat) Done() bool { return s.Status == Complete || s.Status == Errored } - // A Download is one logical job: a URL, a torrent, a magnet. Run blocks in its // own goroutine until the work finishes, fails, or ctx is cancelled. Stat may // be called concurrently at any time and must not block; implementations back diff --git a/go.mod b/go.mod index 6577641..9cb0f23 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.25 require ( github.com/anacrolix/log v0.17.1-0.20251118025802-918f1157b7bb - github.com/anacrolix/missinggo/v2 v2.10.0 github.com/anacrolix/torrent v1.61.0 golang.org/x/sys v0.38.0 golang.org/x/term v0.37.0 @@ -22,6 +21,7 @@ require ( github.com/anacrolix/go-libutp v1.3.2 // indirect github.com/anacrolix/missinggo v1.3.0 // indirect github.com/anacrolix/missinggo/perf v1.0.0 // indirect + github.com/anacrolix/missinggo/v2 v2.10.0 // indirect github.com/anacrolix/mmsg v1.0.1 // indirect github.com/anacrolix/multiless v0.4.0 // indirect github.com/anacrolix/stm v0.5.0 // indirect diff --git a/httpdl/control.go b/httpdl/control.go index 81ff078..af30c42 100644 --- a/httpdl/control.go +++ b/httpdl/control.go @@ -4,7 +4,6 @@ import ( "encoding/json" "os" "path/filepath" - "sort" "sync/atomic" ) @@ -34,7 +33,7 @@ func controlPath(out string) string { return out + ".got" } func snapshot(url string, total int64, etag, lastmod string, segs []seg) control { c := control{URL: url, Total: total, ETag: etag, LastModified: lastmod, Segs: make([]segState, len(segs))} for i := range segs { - c.Segs[i] = segState{segs[i].start, segs[i].endOff(), atomic.LoadInt64(&segs[i].written)} + c.Segs[i] = segState{segs[i].start, segs[i].end, atomic.LoadInt64(&segs[i].written)} } return c } @@ -88,16 +87,15 @@ func loadControl(out, url string, total int64, etag, lastmod string) *control { } // validSegs reports whether segs cover [0,total) with no gap or overlap and a -// sane written count for each. The recorded order is not sorted (a steal appends -// a tail), so we check coverage on a sorted copy. +// sane written count for each. got always writes segments in ascending start +// order, so coverage is checked in place; an out-of-order sidecar (only possible +// from a hand-edited or corrupted file) is rejected, which restarts cleanly. func validSegs(segs []segState, total int64) bool { if total <= 0 || len(segs) == 0 { return false } - sorted := append([]segState(nil), segs...) - sort.Slice(sorted, func(i, j int) bool { return sorted[i].Start < sorted[j].Start }) var next int64 - for _, s := range sorted { + for _, s := range segs { length := s.End - s.Start + 1 if s.Start != next || s.End < s.Start || s.Written < 0 || s.Written > length { return false diff --git a/httpdl/httpdl.go b/httpdl/httpdl.go index c041f09..fc2df30 100644 --- a/httpdl/httpdl.go +++ b/httpdl/httpdl.go @@ -758,7 +758,7 @@ func (d *Download) fetchSeg(ctx context.Context, f *os.File, s *seg, total int64 func (d *Download) fetchOnce(ctx context.Context, f *os.File, s *seg, uri string, total int64) error { reqCtx, cancel := context.WithCancel(ctx) defer cancel() - req, err := d.request(reqCtx, uri, fmt.Sprintf("bytes=%d-%d", s.offset(), s.endOff())) + req, err := d.request(reqCtx, uri, fmt.Sprintf("bytes=%d-%d", s.offset(), s.end)) if err != nil { return err } diff --git a/httpdl/segment.go b/httpdl/segment.go index 00d773b..b8306df 100644 --- a/httpdl/segment.go +++ b/httpdl/segment.go @@ -16,7 +16,6 @@ type seg struct { } func (s *seg) length() int64 { return s.end - s.start + 1 } -func (s *seg) endOff() int64 { return s.end } func (s *seg) done() bool { return atomic.LoadInt64(&s.written) >= s.length() } func (s *seg) addWritten(n int64) { atomic.AddInt64(&s.written, n) } func (s *seg) progress() int64 { return atomic.LoadInt64(&s.written) } diff --git a/httpdl/segment_test.go b/httpdl/segment_test.go index 2160749..973916e 100644 --- a/httpdl/segment_test.go +++ b/httpdl/segment_test.go @@ -201,7 +201,7 @@ func tilesCover(t *testing.T, segs []seg, total int64) { if s.start != next { t.Fatalf("segment gap/overlap: next byte %d, got start %d", next, s.start) } - next = s.endOff() + 1 + next = s.end + 1 } if next != total { t.Fatalf("segments cover %d bytes, want %d", next, total) diff --git a/main.go b/main.go index 5ffb3a5..b8c17b8 100644 --- a/main.go +++ b/main.go @@ -17,7 +17,7 @@ import ( "sort" "strconv" "strings" - "sync" + "sync/atomic" "syscall" "time" @@ -94,8 +94,12 @@ func run(args []string) int { t := time.AfterFunc(time.Duration(stop)*time.Second, cancel) defer t.Stop() } - jobsRef := &jobsHolder{} - go handleSignals(cancel, sessionFile, jobsRef) + var jobsRef atomic.Pointer[[]job] + // publish snapshots the slice header (j is a by-value copy) and stores a + // pointer to it, so the signal handler can read the current jobs lock-free + // without racing the `jobs = append(...)` reassignment in the follow pass. + publish := func(j []job) { jobsRef.Store(&j) } + go handleSignals(cancel, sessionFile, &jobsRef) eng := download.NewEngine(opts.Int("max-concurrent-downloads")) @@ -111,7 +115,7 @@ func run(args []string) int { close(uiDone) } - jobsRef.set(jobs) + publish(jobs) results := runJobs(ctx, eng, jobs) // --follow-torrent: a .torrent fetched over HTTP becomes a BitTorrent @@ -125,7 +129,7 @@ func run(args []string) int { // Index space, so the sort below restores input order deterministically. nInitial := len(jobs) jobs = append(jobs, follow...) - jobsRef.set(jobs) + publish(jobs) followResults := runJobs(ctx, eng, follow) for i := range followResults { followResults[i].Index += nInitial @@ -153,25 +157,6 @@ func run(args []string) int { return report(results) } -// jobsHolder lets the signal handler reach the latest job list for a -// best-effort session save on a forced (second-Ctrl-C) exit. -type jobsHolder struct { - mu sync.Mutex - jobs []job -} - -func (h *jobsHolder) set(jobs []job) { - h.mu.Lock() - h.jobs = jobs - h.mu.Unlock() -} - -func (h *jobsHolder) get() []job { - h.mu.Lock() - defer h.mu.Unlock() - return h.jobs -} - // job pairs a download with the source string that produced it, so we can write // the still-unfinished ones back to a session file. type job struct { @@ -810,14 +795,18 @@ func progressNote(s download.Stat) string { // TERM) cancels the graceful context so run() can wind down and save normally; // a second forces exit, but first writes the session best-effort so the resume // list survives a force-quit. -func handleSignals(cancel context.CancelFunc, sessionFile string, jobs *jobsHolder) { +func handleSignals(cancel context.CancelFunc, sessionFile string, jobs *atomic.Pointer[[]job]) { sig := make(chan os.Signal, 2) signal.Notify(sig, os.Interrupt, syscall.SIGTERM) <-sig // first: cancel the graceful context cancel() <-sig // second: force exit if sessionFile != "" { - if err := saveSession(sessionFile, jobs.get()); err != nil { + var js []job // nil until the first publish; saveSession handles an empty list + if p := jobs.Load(); p != nil { + js = *p + } + if err := saveSession(sessionFile, js); err != nil { fmt.Fprintln(os.Stderr, "got:", err) } } diff --git a/progress/format.go b/progress/format.go index 862adba..9331a5d 100644 --- a/progress/format.go +++ b/progress/format.go @@ -41,12 +41,6 @@ func humanSize(n int64, human bool) string { return fmt.Sprintf("%.0f%ciB", val, suffix) } -// speed formats a bytes-per-second rate. The DL:/UL: fields show the bare -// abbreviated size with no "/s" suffix. -func speed(bytesPerSec int64, human bool) string { - return humanSize(bytesPerSec, human) -} - // secfmt renders a duration as "1h2m3s", appending each unit only when it is // nonzero (but still showing seconds when the whole input is 0): // 3600s->"1h", 120s->"2m", 3720s->"1h2m". A non-positive or absurd diff --git a/progress/format_test.go b/progress/format_test.go index 6eed4e2..cad6485 100644 --- a/progress/format_test.go +++ b/progress/format_test.go @@ -53,16 +53,6 @@ func TestSecfmt(t *testing.T) { } } -func TestSpeedNoSuffix(t *testing.T) { - // The DL:/UL: fields show the bare abbreviated size with no "/s". - if got := speed(1536, true); got != "1.5KiB" { - t.Errorf("speed(1536,true) = %q, want %q", got, "1.5KiB") - } - if got := speed(2048, false); got != "2048B" { - t.Errorf("speed(2048,false) = %q, want %q", got, "2048B") - } -} - func TestPercent(t *testing.T) { if got := percent(50, 100); got != 50 { t.Errorf("percent(50,100) = %d, want 50", got) diff --git a/progress/progress.go b/progress/progress.go index cf087b4..5c3c97c 100644 --- a/progress/progress.go +++ b/progress/progress.go @@ -147,10 +147,10 @@ func (r *Reporter) lineOne(s download.Stat) string { fmt.Fprintf(&b, " SD:%d", s.Seeders) } if !seeding { - fmt.Fprintf(&b, " DL:%s", speed(dl, r.human)) + fmt.Fprintf(&b, " DL:%s", humanSize(dl, r.human)) } if seeding || ul > 0 { - fmt.Fprintf(&b, " UL:%s", speed(ul, r.human)) + fmt.Fprintf(&b, " UL:%s", humanSize(ul, r.human)) } if !seeding && s.Total > 0 && dl > 0 { fmt.Fprintf(&b, " ETA:%s", secfmt(etaDuration(s.Total-s.Completed, dl))) @@ -227,7 +227,7 @@ func (r *Reporter) summary(stats []download.Stat) string { stalled++ } } - line := fmt.Sprintf("%d active DL:%s UL:%s", len(stats)-waiting, speed(totDL, r.human), speed(totUL, r.human)) + line := fmt.Sprintf("%d active DL:%s UL:%s", len(stats)-waiting, humanSize(totDL, r.human), humanSize(totUL, r.human)) if waiting > 0 { line += fmt.Sprintf(" (%d waiting)", waiting) } @@ -254,13 +254,13 @@ func (r *Reporter) rowMetric(s download.Stat, dl, ul int64) (metric, tail string case download.Waiting: return "queued", "" case download.Seeding: - return "seeding", "UL:" + speed(ul, r.human) + return "seeding", "UL:" + humanSize(ul, r.human) case download.Complete: return "done", "" case download.Errored: return "failed", "" default: - metric = "DL:" + speed(dl, r.human) + metric = "DL:" + humanSize(dl, r.human) switch { case dl > 0 && s.Total > 0: tail = secfmt(etaDuration(s.Total-s.Completed, dl))