Open joonas-fi opened 2 years ago
stapelberg
commented
2 years ago The unused and predeclared are too aggressive, I think.
For the others, can you send a PR to fix (or exempt them where appropriate) them all and set up this static analyzer in GitHub Actions so that the code doesn’t regress please?
joonas-fi
commented
2 years ago Sure, no problem. But I'd like to get the previous stuff resolved first in https://github.com/gokrazy/rsync/issues/12
(Too much parallel work brings overhead due to expected merge conflicts that the "internal -> something else" rename will bring)
joonas-fi
commented
2 years ago I updated the issue text to contain list of tasks.
joonas-fi
commented
2 years ago There seems to be ready GitHub action one can use: https://github.com/golangci/golangci-lint-action
Tasks:
I ran golangci-lint, with configuration I've settled on for my projects. Here's the current issues it found:
Lint report
``` pkg/rsyncd/rsyncd.go:76:6: `sumBuf` is unused (deadcode) type sumBuf struct { ^ pkg/maincmd/unprivileged_linux.go:10:6: `runAsUnprivilegedUser` is unused (deadcode) func runAsUnprivilegedUser(cmd *exec.Cmd) { ^ pkg/rsyncchecksum/rsyncchecksum.go:52:14: Error return value of `binary.Write` is not checked (errcheck) binary.Write(h, binary.LittleEndian, seed) ^ pkg/rsyncwire/wire.go:96:14: Error return value of `binary.Write` is not checked (errcheck) binary.Write(&b.buf, binary.LittleEndian, data) ^ pkg/rsyncwire/wire.go:100:14: Error return value of `binary.Write` is not checked (errcheck) binary.Write(&b.buf, binary.LittleEndian, data) ^ pkg/rsyncwire/wire.go:115:16: Error return value of `io.WriteString` is not checked (errcheck) io.WriteString(&b.buf, data) ^ pkg/anonssh/anonssh.go:166:14: Error return value of `req.Reply` is not checked (errcheck) req.Reply(false, errmsg) ^ pkg/anonssh/anonssh.go:167:18: Error return value of `channel.Write` is not checked (errcheck) channel.Write(errmsg) ^ pkg/anonssh/anonssh.go:180:17: Error return value of `newChan.Reject` is not checked (errcheck) newChan.Reject(ssh.UnknownChannelType, fmt.Sprintf("unknown channel type: %q", t)) ^ pkg/receivermaincmd/receiver.go:98:19: Error return value of `out.Cleanup` is not checked (errcheck) defer out.Cleanup() ^ pkg/rsyncd/rsyncd.go:198:17: Error return value of `io.WriteString` is not checked (errcheck) io.WriteString(cwr, s.formatModuleList()) ^ pkg/rsyncd/rsyncd.go:199:17: Error return value of `io.WriteString` is not checked (errcheck) io.WriteString(cwr, "@RSYNCD: EXIT\n") ^ pkg/rsyncd/rsyncd.go:253:15: Error return value of `mpx.WriteMsg` is not checked (errcheck) mpx.WriteMsg(rsyncwire.MsgError, []byte(fmt.Sprintf("gokr-rsync [sender]: %v\n", err))) ^ pkg/rsyncd/rsyncd.go:312:16: Error return value of `mpx.WriteMsg` is not checked (errcheck) mpx.WriteMsg(rsyncwire.MsgError, []byte(fmt.Sprintf("gokr-rsync [sender]: %v\n", err))) ^ pkg/rsynctest/rsynctest.go:109:15: Error return value of `srv.Serve` is not checked (errcheck) go srv.Serve(context.Background(), ts.listener) ^ pkg/anonssh/anonssh.go:60:3: commentFormatting: put a space between `//` and comment text (gocritic) //s.env = append(s.env, fmt.Sprintf("%s=%s", r.VariableName, r.VariableValue)) ^ pkg/receivermaincmd/receivermaincmd.go:169:10: elseif: can replace 'else {if cond {}}' with 'else if cond {}' (gocritic) } else { ^ pkg/rsyncd/match.go:126:6: commentFormatting: put a space between `//` and comment text (gocritic) //falseAlarms++ ^ pkg/rsyncd/match.go:181:3: commentFormatting: put a space between `//` and comment text (gocritic) //log.Printf("null_tag, k=%d", k) ^ cmd/gokr-rsyncd/rsyncd.go:25:3: exitAfterDefer: log.Fatal will exit, and `defer cancel()` will not run (gocritic) log.Fatal(err) ^ cmd/libuser/main.go:23:3: exitAfterDefer: log.Fatal will exit, and `defer cancel()` will not run (gocritic) log.Fatal(err) ^ pkg/rsyncchecksum/checksum_test.go:28:12: G306: Expect WriteFile permissions to be 0600 or less (gosec) if err := ioutil.WriteFile(large, content, 0644); err != nil { ^ pkg/receivermaincmd/receivermaincmd.go:277:9: G204: Subprocess launched with a potential tainted input or cmd arguments (gosec) ssh := exec.Command(args[0], args[1:]...) ^ pkg/maincmd/maincmd.go:18:2: G108: Profiling endpoint is automatically exposed on /debug/pprof (gosec) _ "net/http/pprof" ^ pkg/maincmd/writetest.go:12:12: G306: Expect WriteFile permissions to be 0600 or less (gosec) if err := ioutil.WriteFile(fn, []byte("gokr-rsyncd creates this file to prevent misconfigurations. if you see this file, it means gokr-rsyncd unexpectedly was started with too many privileges"), 0644); err == nil { ^ pkg/rsynctest/rsynctest.go:251:12: G306: Expect WriteFile permissions to be 0600 or less (gosec) if err := ioutil.WriteFile(large, content, 0644); err != nil { ^ pkg/anonssh/anonssh.go:43:2: `env` is unused (structcheck) env []string ^ pkg/anonssh/anonssh.go:44:2: `ptyf` is unused (structcheck) ptyf *os.File ^ pkg/anonssh/anonssh.go:45:2: `ttyf` is unused (structcheck) ttyf *os.File ^ pkg/anonssh/anonssh.go:27:2: `cfg` is unused (structcheck) cfg *config.Config ^ pkg/rsyncd/rsyncd.go:80:2: `sum1` is unused (structcheck) sum1 uint32 ^ pkg/rsyncd/rsyncd.go:81:2: `sum2` is unused (structcheck) sum2 [16]byte ^ pkg/rsyncd/rsyncd.go:77:2: `offset` is unused (structcheck) offset int64 ^ pkg/rsyncd/rsyncd.go:78:2: `len` is unused (structcheck) len int64 ^ pkg/rsyncd/rsyncd.go:79:2: `index` is unused (structcheck) index int32 ^ pkg/rsyncd/token.go:39:36: unnecessary conversion (unconvert) return st.conn.WriteInt32(-(int32(token) + 1)) ^ pkg/anonssh/anonssh.go:50:27: `(*session).request` - `ctx` is unused (unparam) func (s *session) request(ctx context.Context, req *ssh.Request) error { ^ pkg/receivermaincmd/generator.go:46:66: (*recvTransfer).skipFile - result 1 (error) is always nil (unparam) func (rt *recvTransfer) skipFile(f *file, st os.FileInfo) (bool, error) { ^ pkg/receivermaincmd/receivermaincmd.go:239:12: `doCmd` - `osenv` is unused (unparam) func doCmd(osenv osenv, opts *Opts, machine, user, path string, daemonConnection int) (io.ReadCloser, io.WriteCloser, error) { ^ pkg/rsyncd/flist.go:17:38: `(*sendTransfer).sendFileList` - `c` is unused (unparam) func (st *sendTransfer) sendFileList(c *rsyncwire.Conn, mod config.Module, opts *Opts, paths []string) (*fileList, error) { ^ pkg/rsyncd/token.go:45:79: `(*sendTransfer).sendToken` - `toklen` is unused (unparam) func (st *sendTransfer) sendToken(f *os.File, i int32, offset int64, n int64, toklen int64) error { ^ pkg/rsyncd/match.go:160:5: unreachable: unreachable code (govet) offset += head.Sums[i].Len - 1 ^ pkg/rsyncd/match.go:182:3: unreachable: unreachable code (govet) readk := k + 1 ^ pkg/rsyncchecksum/rsyncchecksum.go:26:2: variable len has same name as predeclared identifier (predeclared) len := len(buf) ^ pkg/rsynccommon/rsynccommon.go:14:21: param len has same name as predeclared identifier (predeclared) func SumSizesSqroot(len int64) rsync.SumHead { ^ pkg/receivermaincmd/generator.go:263:58: param len has same name as predeclared identifier (predeclared) func (rt *recvTransfer) generateAndSendSums(in *os.File, len int64) error { ^ pkg/receivermaincmd/receiver.go:124:3: variable len has same name as predeclared identifier (predeclared) len := sh.BlockLength ^ pkg/maincmd/maincmd.go:44:3: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) fmt.Fprintf(stderr, opt.Help()) ^ pkg/receivermaincmd/generator.go:182:2: SA9003: empty branch (staticcheck) if rt.opts.PreserveHardlinks { ^ pkg/receivermaincmd/generator.go:89:2: SA4006: this value of `st` is never used (staticcheck) st, err = rt.setUid(f, local, st) ^ ``` Some of these are non-issues, but some seem clear bugs like this one: ``` pkg/maincmd/maincmd.go:44:3: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) fmt.Fprintf(stderr, opt.Help()) ```Would you like me to submit a PR to fix all or some of these complaints?