rsync
rsync copied to clipboard
Published
20 hours ago •
gokrazy
gokrazy
Issues found by static analysis
Tasks:
- [x] Send PR for most clear issues
- [x] Have the above PR be merged
- [ ] Agree on a lint rule set
- [ ] Fix the rest of the issues found by agreed-on lint rule set
- [ ] Set up GitHub action so there will be no new regressions
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?
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?
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)
There seems to be ready GitHub action one can use: https://github.com/golangci/golangci-lint-action