From 60fcaebfdb8d8a4ce6ff7fb2aada6226ce5e9964 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 28 Apr 2015 22:32:10 +0200 Subject: [PATCH] Run vet and lint. Make us lint clean. --- build.go | 55 ++++++++++++++++++++++++--- build.sh | 21 +++++++++- cmd/syncthing/gui.go | 12 +++--- cmd/syncthing/gui_auth.go | 4 +- cmd/syncthing/gui_csrf.go | 2 +- cmd/syncthing/monitor.go | 6 +-- internal/config/config.go | 32 ++++++++-------- internal/config/wrapper.go | 6 +-- internal/db/blockmap.go | 17 ++++----- internal/db/leveldb.go | 10 ++--- internal/fnmatch/fnmatch.go | 4 +- internal/ignore/ignore.go | 3 +- internal/model/model.go | 33 ++++++++-------- internal/model/model_test.go | 4 +- internal/model/progressemitter.go | 15 ++++---- internal/osutil/osutil.go | 5 ++- internal/scanner/blocks.go | 4 +- internal/scanner/walk.go | 16 ++++---- internal/sync/sync_test.go | 6 +-- internal/upgrade/upgrade_common.go | 4 +- internal/upgrade/upgrade_supported.go | 3 +- internal/upnp/upnp.go | 42 ++++++++++---------- internal/versioner/external.go | 6 +-- internal/versioner/simple.go | 6 +-- internal/versioner/staggered.go | 6 +-- 25 files changed, 189 insertions(+), 133 deletions(-) diff --git a/build.go b/build.go index 2287f28e7..f5b275609 100644 --- a/build.go +++ b/build.go @@ -78,6 +78,11 @@ func main() { tags = []string{"noupgrade"} } install("./cmd/...", tags) + + vet("./cmd/syncthing") + vet("./internal/...") + lint("./cmd/syncthing") + lint("./internal/...") return } @@ -103,8 +108,7 @@ func main() { build(pkg, tags) case "test": - pkg := "./..." - test(pkg) + test("./...") case "assets": assets() @@ -130,6 +134,14 @@ func main() { case "clean": clean() + case "vet": + vet("./cmd/syncthing") + vet("./internal/...") + + case "lint": + lint("./cmd/syncthing") + lint("./internal/...") + default: log.Fatalf("Unknown command %q", cmd) } @@ -437,10 +449,7 @@ func run(cmd string, args ...string) []byte { func runError(cmd string, args ...string) ([]byte, error) { ecmd := exec.Command(cmd, args...) bs, err := ecmd.CombinedOutput() - if err != nil { - return nil, err - } - return bytes.TrimSpace(bs), nil + return bytes.TrimSpace(bs), err } func runPrint(cmd string, args ...string) { @@ -615,3 +624,37 @@ func md5File(file string) error { return out.Close() } + +func vet(pkg string) { + bs, err := runError("go", "vet", pkg) + if err != nil && err.Error() == "exit status 3" || bytes.Contains(bs, []byte("no such tool \"vet\"")) { + // Go said there is no go vet + log.Println(`- No go vet, no vetting. Try "go get -u golang.org/x/tools/cmd/vet".`) + return + } + + falseAlarmComposites := regexp.MustCompile("composite literal uses unkeyed fields") + exitStatus := regexp.MustCompile("exit status 1") + for _, line := range bytes.Split(bs, []byte("\n")) { + if falseAlarmComposites.Match(line) || exitStatus.Match(line) { + continue + } + log.Printf("%s", line) + } +} + +func lint(pkg string) { + bs, err := runError("golint", pkg) + if err != nil { + log.Println(`- No golint, not linting. Try "go get -u github.com/golang/lint/golint".`) + return + } + + analCommentPolicy := regexp.MustCompile(`exported (function|method|const|type|var) [^\s]+ should have comment`) + for _, line := range bytes.Split(bs, []byte("\n")) { + if analCommentPolicy.Match(line) { + continue + } + log.Printf("%s", line) + } +} diff --git a/build.sh b/build.sh index b3c42c87b..58f49d359 100755 --- a/build.sh +++ b/build.sh @@ -118,8 +118,6 @@ case "${1:-default}" in -e "STTRACE=$STTRACE" \ syncthing/build:latest \ sh -c './build.sh clean \ - && go tool vet -composites=false cmd/*/*.go internal/*/*.go \ - && ( golint ./cmd/... ; golint ./internal/... ) | egrep -v "comment on exported|should have comment" \ && ./build.sh all \ && STTRACE=all ./build.sh test-cov' ;; @@ -138,6 +136,25 @@ case "${1:-default}" in && git clean -fxd .' ;; + docker-lint) + docker run --rm -h syncthing-builder -u $(id -u) -t \ + -v $(pwd):/go/src/github.com/syncthing/syncthing \ + -w /go/src/github.com/syncthing/syncthing \ + -e "STTRACE=$STTRACE" \ + syncthing/build:latest \ + sh -euxc 'go run build.go lint' + ;; + + + docker-vet) + docker run --rm -h syncthing-builder -u $(id -u) -t \ + -v $(pwd):/go/src/github.com/syncthing/syncthing \ + -w /go/src/github.com/syncthing/syncthing \ + -e "STTRACE=$STTRACE" \ + syncthing/build:latest \ + sh -euxc 'go run build.go vet' + ;; + *) echo "Unknown build command $1" ;; diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index c9847bc63..1bef05c4a 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -45,16 +45,16 @@ type guiError struct { } var ( - configInSync = true - guiErrors = []guiError{} - guiErrorsMut sync.Mutex = sync.NewMutex() - startTime = time.Now() + configInSync = true + guiErrors = []guiError{} + guiErrorsMut = sync.NewMutex() + startTime = time.Now() eventSub *events.BufferedSubscription ) var ( lastEventRequest time.Time - lastEventRequestMut sync.Mutex = sync.NewMutex() + lastEventRequestMut = sync.NewMutex() ) func startGUI(cfg config.GUIConfiguration, assetDir string, m *model.Model) error { @@ -538,7 +538,7 @@ func flushResponse(s string, w http.ResponseWriter) { } var cpuUsagePercent [10]float64 // The last ten seconds -var cpuUsageLock sync.RWMutex = sync.NewRWMutex() +var cpuUsageLock = sync.NewRWMutex() func restGetSystemStatus(w http.ResponseWriter, r *http.Request) { var m runtime.MemStats diff --git a/cmd/syncthing/gui_auth.go b/cmd/syncthing/gui_auth.go index 21ad06dc6..e95e58d8d 100644 --- a/cmd/syncthing/gui_auth.go +++ b/cmd/syncthing/gui_auth.go @@ -20,8 +20,8 @@ import ( ) var ( - sessions = make(map[string]bool) - sessionsMut sync.Mutex = sync.NewMutex() + sessions = make(map[string]bool) + sessionsMut = sync.NewMutex() ) func basicAuthAndSessionMiddleware(cfg config.GUIConfiguration, next http.Handler) http.Handler { diff --git a/cmd/syncthing/gui_csrf.go b/cmd/syncthing/gui_csrf.go index 721adf304..a30d1faad 100644 --- a/cmd/syncthing/gui_csrf.go +++ b/cmd/syncthing/gui_csrf.go @@ -19,7 +19,7 @@ import ( ) var csrfTokens []string -var csrfMut sync.Mutex = sync.NewMutex() +var csrfMut = sync.NewMutex() // Check for CSRF token on /rest/ URLs. If a correct one is not given, reject // the request with 403. For / and /index.html, set a new CSRF cookie if none diff --git a/cmd/syncthing/monitor.go b/cmd/syncthing/monitor.go index 59b4f4b78..9743adcc9 100644 --- a/cmd/syncthing/monitor.go +++ b/cmd/syncthing/monitor.go @@ -22,9 +22,9 @@ import ( ) var ( - stdoutFirstLines []string // The first 10 lines of stdout - stdoutLastLines []string // The last 50 lines of stdout - stdoutMut sync.Mutex = sync.NewMutex() + stdoutFirstLines []string // The first 10 lines of stdout + stdoutLastLines []string // The last 50 lines of stdout + stdoutMut = sync.NewMutex() ) const ( diff --git a/internal/config/config.go b/internal/config/config.go index 6291835ef..c7d3bd37f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -45,28 +45,28 @@ type Configuration struct { OriginalVersion int `xml:"-" json:"-"` // The version we read from disk, before any conversion } -func (orig Configuration) Copy() Configuration { - c := orig +func (cfg Configuration) Copy() Configuration { + newCfg := cfg // Deep copy FolderConfigurations - c.Folders = make([]FolderConfiguration, len(orig.Folders)) - for i := range c.Folders { - c.Folders[i] = orig.Folders[i].Copy() + newCfg.Folders = make([]FolderConfiguration, len(cfg.Folders)) + for i := range newCfg.Folders { + newCfg.Folders[i] = cfg.Folders[i].Copy() } // Deep copy DeviceConfigurations - c.Devices = make([]DeviceConfiguration, len(orig.Devices)) - for i := range c.Devices { - c.Devices[i] = orig.Devices[i].Copy() + newCfg.Devices = make([]DeviceConfiguration, len(cfg.Devices)) + for i := range newCfg.Devices { + newCfg.Devices[i] = cfg.Devices[i].Copy() } - c.Options = orig.Options.Copy() + newCfg.Options = cfg.Options.Copy() // DeviceIDs are values - c.IgnoredDevices = make([]protocol.DeviceID, len(orig.IgnoredDevices)) - copy(c.IgnoredDevices, orig.IgnoredDevices) + newCfg.IgnoredDevices = make([]protocol.DeviceID, len(cfg.IgnoredDevices)) + copy(newCfg.IgnoredDevices, cfg.IgnoredDevices) - return c + return newCfg } type FolderConfiguration struct { @@ -89,10 +89,10 @@ type FolderConfiguration struct { deviceIDs []protocol.DeviceID } -func (orig FolderConfiguration) Copy() FolderConfiguration { - c := orig - c.Devices = make([]FolderDeviceConfiguration, len(orig.Devices)) - copy(c.Devices, orig.Devices) +func (f FolderConfiguration) Copy() FolderConfiguration { + c := f + c.Devices = make([]FolderDeviceConfiguration, len(f.Devices)) + copy(c.Devices, f.Devices) return c } diff --git a/internal/config/wrapper.go b/internal/config/wrapper.go index aa7d96aa3..ba7dfce20 100644 --- a/internal/config/wrapper.go +++ b/internal/config/wrapper.go @@ -156,7 +156,7 @@ func (w *Wrapper) SetDevice(dev DeviceConfiguration) { w.replaces <- w.cfg.Copy() } -// Devices returns a map of folders. Folder structures should not be changed, +// Folders returns a map of folders. Folder structures should not be changed, // other than for the purpose of updating via SetFolder(). func (w *Wrapper) Folders() map[string]FolderConfiguration { w.mut.Lock() @@ -220,8 +220,8 @@ func (w *Wrapper) SetGUI(gui GUIConfiguration) { w.replaces <- w.cfg.Copy() } -// Returns whether or not connection attempts from the given device should be -// silently ignored. +// IgnoredDevice returns whether or not connection attempts from the given +// device should be silently ignored. func (w *Wrapper) IgnoredDevice(id protocol.DeviceID) bool { w.mut.Lock() defer w.mut.Unlock() diff --git a/internal/db/blockmap.go b/internal/db/blockmap.go index 1be8c1bbe..496e971be 100644 --- a/internal/db/blockmap.go +++ b/internal/db/blockmap.go @@ -10,7 +10,6 @@ // depending on who calls us. We transform paths to wire-format (NFC and // slashes) on the way to the database, and transform to native format // (varying separator and encoding) on the way back out. - package db import ( @@ -131,7 +130,7 @@ func NewBlockFinder(db *leveldb.DB, cfg *config.Wrapper) *BlockFinder { return f } -// Implements config.Handler interface +// Changed implements config.Handler interface func (f *BlockFinder) Changed(cfg config.Configuration) error { folders := make([]string, len(cfg.Folders)) for i, folder := range cfg.Folders { @@ -147,11 +146,11 @@ func (f *BlockFinder) Changed(cfg config.Configuration) error { return nil } -// An iterator function which iterates over all matching blocks for the given -// hash. The iterator function has to return either true (if they are happy with -// the block) or false to continue iterating for whatever reason. -// The iterator finally returns the result, whether or not a satisfying block -// was eventually found. +// Iterate takes an iterator function which iterates over all matching blocks +// for the given hash. The iterator function has to return either true (if +// they are happy with the block) or false to continue iterating for whatever +// reason. The iterator finally returns the result, whether or not a +// satisfying block was eventually found. func (f *BlockFinder) Iterate(hash []byte, iterFn func(string, string, int32) bool) bool { f.mut.RLock() folders := f.folders @@ -172,8 +171,8 @@ func (f *BlockFinder) Iterate(hash []byte, iterFn func(string, string, int32) bo return false } -// A method for repairing incorrect blockmap entries, removes the old entry -// and replaces it with a new entry for the given block +// Fix repairs incorrect blockmap entries, removing the old entry and +// replacing it with a new entry for the given block func (f *BlockFinder) Fix(folder, file string, index int32, oldHash, newHash []byte) error { buf := make([]byte, 4) binary.BigEndian.PutUint32(buf, uint32(index)) diff --git a/internal/db/leveldb.go b/internal/db/leveldb.go index 241899cd6..be4f420c6 100644 --- a/internal/db/leveldb.go +++ b/internal/db/leveldb.go @@ -25,7 +25,7 @@ import ( var ( clockTick int64 - clockMut sync.Mutex = sync.NewMutex() + clockMut = sync.NewMutex() ) func clock(v int64) int64 { @@ -976,11 +976,11 @@ func unmarshalTrunc(bs []byte, truncate bool) (FileIntf, error) { var tf FileInfoTruncated err := tf.UnmarshalXDR(bs) return tf, err - } else { - var tf protocol.FileInfo - err := tf.UnmarshalXDR(bs) - return tf, err } + + var tf protocol.FileInfo + err := tf.UnmarshalXDR(bs) + return tf, err } func ldbCheckGlobals(db *leveldb.DB, folder []byte) { diff --git a/internal/fnmatch/fnmatch.go b/internal/fnmatch/fnmatch.go index 0882f30f7..0147be886 100644 --- a/internal/fnmatch/fnmatch.go +++ b/internal/fnmatch/fnmatch.go @@ -75,8 +75,8 @@ func Convert(pattern string, flags int) (*regexp.Regexp, error) { return regexp.Compile(pattern) } -// Matches the pattern against the string, with the given flags, -// and returns true if the match is successful. +// Match matches the pattern against the string, with the given flags, and +// returns true if the match is successful. func Match(pattern, s string, flags int) (bool, error) { exp, err := Convert(pattern, flags) if err != nil { diff --git a/internal/ignore/ignore.go b/internal/ignore/ignore.go index ca216ea31..70f77c5b1 100644 --- a/internal/ignore/ignore.go +++ b/internal/ignore/ignore.go @@ -30,9 +30,8 @@ type Pattern struct { func (p Pattern) String() string { if p.include { return p.match.String() - } else { - return "(?exclude)" + p.match.String() } + return "(?exclude)" + p.match.String() } type Matcher struct { diff --git a/internal/model/model.go b/internal/model/model.go index 061625a80..68ab469c6 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -125,15 +125,16 @@ func NewModel(cfg *config.Wrapper, id protocol.DeviceID, deviceName, clientName, return m } -// Starts deadlock detector on the models locks which causes panics in case -// the locks cannot be acquired in the given timeout period. +// StartDeadlockDetector starts a deadlock detector on the models locks which +// causes panics in case the locks cannot be acquired in the given timeout +// period. func (m *Model) StartDeadlockDetector(timeout time.Duration) { l.Infof("Starting deadlock detector with %v timeout", timeout) deadlockDetect(m.fmut, timeout) deadlockDetect(m.pmut, timeout) } -// StartRW starts read/write processing on the current model. When in +// StartFolderRW starts read/write processing on the current model. When in // read/write mode the model will attempt to keep in sync with the cluster by // pulling needed files from peer devices. func (m *Model) StartFolderRW(folder string) { @@ -166,9 +167,9 @@ func (m *Model) StartFolderRW(folder string) { go p.Serve() } -// StartRO starts read only processing on the current model. When in -// read only mode the model will announce files to the cluster but not -// pull in any external changes. +// StartFolderRO starts read only processing on the current model. When in +// read only mode the model will announce files to the cluster but not pull in +// any external changes. func (m *Model) StartFolderRO(folder string) { m.fmut.Lock() cfg, ok := m.folderCfgs[folder] @@ -243,7 +244,7 @@ func (m *Model) ConnectionStats() map[string]interface{} { return res } -// Returns statistics about each device +// DeviceStatistics returns statistics about each device func (m *Model) DeviceStatistics() map[string]stats.DeviceStatistics { var res = make(map[string]stats.DeviceStatistics) for id := range m.cfg.Devices() { @@ -252,7 +253,7 @@ func (m *Model) DeviceStatistics() map[string]stats.DeviceStatistics { return res } -// Returns statistics about each folder +// FolderStatistics returns statistics about each folder func (m *Model) FolderStatistics() map[string]stats.FolderStatistics { var res = make(map[string]stats.FolderStatistics) for id := range m.cfg.Folders() { @@ -261,7 +262,8 @@ func (m *Model) FolderStatistics() map[string]stats.FolderStatistics { return res } -// Returns the completion status, in percent, for the given device and folder. +// Completion returns the completion status, in percent, for the given device +// and folder. func (m *Model) Completion(device protocol.DeviceID, folder string) float64 { var tot int64 @@ -375,9 +377,9 @@ func (m *Model) NeedSize(folder string) (nfiles int, bytes int64) { return } -// NeedFiles returns paginated list of currently needed files in progress, queued, -// and to be queued on next puller iteration, as well as the total number of -// files currently needed. +// NeedFolderFiles returns paginated list of currently needed files in +// progress, queued, and to be queued on next puller iteration, as well as the +// total number of files currently needed. func (m *Model) NeedFolderFiles(folder string, page, perpage int) ([]db.FileInfoTruncated, []db.FileInfoTruncated, []db.FileInfoTruncated, int) { m.fmut.RLock() defer m.fmut.RUnlock() @@ -1535,7 +1537,7 @@ func (m *Model) Availability(folder, file string) []protocol.DeviceID { return availableDevices } -// Bump the given files priority in the job queue +// BringToFront bumps the given files priority in the job queue. func (m *Model) BringToFront(folder, file string) { m.pmut.RLock() defer m.pmut.RUnlock() @@ -1546,9 +1548,8 @@ func (m *Model) BringToFront(folder, file string) { } } -// Returns current folder error, or nil if the folder is healthy. -// Updates the Invalid field on the folder configuration struct, and emits a -// ConfigSaved event which causes a GUI refresh. +// CheckFolderHealth checks the folder for common errors and returns the +// current folder error, or nil if the folder is healthy. func (m *Model) CheckFolderHealth(id string) error { folder, ok := m.cfg.Folders()[id] if !ok { diff --git a/internal/model/model_test.go b/internal/model/model_test.go index 0e85ae0aa..3d4d36c9a 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -760,7 +760,7 @@ func TestGlobalDirectoryTree(t *testing.T) { m.AddFolder(defaultFolderConfig) b := func(isfile bool, path ...string) protocol.FileInfo { - var flags uint32 = protocol.FlagDirectory + flags := uint32(protocol.FlagDirectory) blocks := []protocol.BlockInfo{} if isfile { flags = 0 @@ -1009,7 +1009,7 @@ func TestGlobalDirectorySelfFixing(t *testing.T) { m.AddFolder(defaultFolderConfig) b := func(isfile bool, path ...string) protocol.FileInfo { - var flags uint32 = protocol.FlagDirectory + flags := uint32(protocol.FlagDirectory) blocks := []protocol.BlockInfo{} if isfile { flags = 0 diff --git a/internal/model/progressemitter.go b/internal/model/progressemitter.go index ca2d374ed..bb31b4778 100755 --- a/internal/model/progressemitter.go +++ b/internal/model/progressemitter.go @@ -27,8 +27,8 @@ type ProgressEmitter struct { stop chan struct{} } -// Creates a new progress emitter which emits DownloadProgress events every -// interval. +// NewProgressEmitter creates a new progress emitter which emits +// DownloadProgress events every interval. func NewProgressEmitter(cfg *config.Wrapper) *ProgressEmitter { t := &ProgressEmitter{ stop: make(chan struct{}), @@ -42,8 +42,8 @@ func NewProgressEmitter(cfg *config.Wrapper) *ProgressEmitter { return t } -// Starts progress emitter which starts emitting DownloadProgress events as -// the progress happens. +// Serve starts the progress emitter which starts emitting DownloadProgress +// events as the progress happens. func (t *ProgressEmitter) Serve() { for { select { @@ -81,7 +81,8 @@ func (t *ProgressEmitter) Serve() { } } -// Interface method to handle configuration changes +// Changed implements the config.Handler Interface to handle configuration +// changes func (t *ProgressEmitter) Changed(cfg config.Configuration) error { t.mut.Lock() defer t.mut.Unlock() @@ -93,7 +94,7 @@ func (t *ProgressEmitter) Changed(cfg config.Configuration) error { return nil } -// Stops the emitter. +// Stop stops the emitter. func (t *ProgressEmitter) Stop() { t.stop <- struct{}{} } @@ -122,7 +123,7 @@ func (t *ProgressEmitter) Deregister(s *sharedPullerState) { delete(t.registry, filepath.Join(s.folder, s.file.Name)) } -// Returns number of bytes completed in the given folder. +// BytesCompleted returns the number of bytes completed in the given folder. func (t *ProgressEmitter) BytesCompleted(folder string) (bytes int64) { t.mut.Lock() defer t.mut.Unlock() diff --git a/internal/osutil/osutil.go b/internal/osutil/osutil.go index fb313440c..093232880 100644 --- a/internal/osutil/osutil.go +++ b/internal/osutil/osutil.go @@ -23,7 +23,7 @@ var ErrNoHome = errors.New("No home directory found - set $HOME (or the platform // Try to keep this entire operation atomic-like. We shouldn't be doing this // often enough that there is any contention on this lock. -var renameLock sync.Mutex = sync.NewMutex() +var renameLock = sync.NewMutex() // TryRename renames a file, leaving source file intact in case of failure. // Tries hard to succeed on various systems by temporarily tweaking directory @@ -89,7 +89,8 @@ func InWritableDir(fn func(string) error, path string) error { return fn(path) } -// On Windows, removes the read-only attribute from the target prior deletion. +// Remove removes the given path. On Windows, removes the read-only attribute +// from the target prior to deletion. func Remove(path string) error { if runtime.GOOS == "windows" { info, err := os.Stat(path) diff --git a/internal/scanner/blocks.go b/internal/scanner/blocks.go index b00dee3c9..bd191f238 100644 --- a/internal/scanner/blocks.go +++ b/internal/scanner/blocks.go @@ -59,7 +59,7 @@ func Blocks(r io.Reader, blocksize int, sizehint int64) ([]protocol.BlockInfo, e return blocks, nil } -// Set the Offset field on each block +// PopulateOffsets sets the Offset field on each block func PopulateOffsets(blocks []protocol.BlockInfo) { var offset int64 for i := range blocks { @@ -139,7 +139,7 @@ func VerifyBuffer(buf []byte, block protocol.BlockInfo) ([]byte, error) { return hash, nil } -// BlockEqual returns whether two slices of blocks are exactly the same hash +// BlocksEqual returns whether two slices of blocks are exactly the same hash // and index pair wise. func BlocksEqual(src, tgt []protocol.BlockInfo) bool { if len(tgt) != len(src) { diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 5fcfeb0ed..41f67e88b 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -379,15 +379,15 @@ func PermsEqual(a, b uint32) bool { } } -// If the target is missing, Unix never knows what type of symlink it is -// and Windows always knows even if there is no target. -// Which means that without this special check a Unix node would be fighting -// with a Windows node about whether or not the target is known. -// Basically, if you don't know and someone else knows, just accept it. -// The fact that you don't know means you are on Unix, and on Unix you don't -// really care what the target type is. The moment you do know, and if something -// doesn't match, that will propogate throught the cluster. func SymlinkTypeEqual(disk, index uint32) bool { + // If the target is missing, Unix never knows what type of symlink it is + // and Windows always knows even if there is no target. Which means that + // without this special check a Unix node would be fighting with a Windows + // node about whether or not the target is known. Basically, if you don't + // know and someone else knows, just accept it. The fact that you don't + // know means you are on Unix, and on Unix you don't really care what the + // target type is. The moment you do know, and if something doesn't match, + // that will propogate throught the cluster. if disk&protocol.FlagSymlinkMissingTarget != 0 && index&protocol.FlagSymlinkMissingTarget == 0 { return true } diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index 6fda6275e..536db3493 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -58,7 +58,7 @@ func TestMutex(t *testing.T) { threshold = logThreshold msgmut := sync.Mutex{} - messages := make([]string, 0) + var messages []string l.AddHandler(logger.LevelDebug, func(_ logger.LogLevel, message string) { msgmut.Lock() @@ -91,7 +91,7 @@ func TestRWMutex(t *testing.T) { threshold = logThreshold msgmut := sync.Mutex{} - messages := make([]string, 0) + var messages []string l.AddHandler(logger.LevelDebug, func(_ logger.LogLevel, message string) { msgmut.Lock() @@ -149,7 +149,7 @@ func TestWaitGroup(t *testing.T) { threshold = logThreshold msgmut := sync.Mutex{} - messages := make([]string, 0) + var messages []string l.AddHandler(logger.LevelDebug, func(_ logger.LogLevel, message string) { msgmut.Lock() diff --git a/internal/upgrade/upgrade_common.go b/internal/upgrade/upgrade_common.go index 55ef18c3f..99b8f03ac 100644 --- a/internal/upgrade/upgrade_common.go +++ b/internal/upgrade/upgrade_common.go @@ -40,7 +40,6 @@ func init() { upgradeUnlocked <- true } -// A wrapper around actual implementations func To(rel Release) error { select { case <-upgradeUnlocked: @@ -60,7 +59,6 @@ func To(rel Release) error { } } -// A wrapper around actual implementations func ToURL(url string) error { select { case <-upgradeUnlocked: @@ -90,7 +88,7 @@ const ( MajorNewer = 2 // Newer by a major version (x in x.y.z or 0.x.y). ) -// Returns a relation describing how a compares to b. +// CompareVersions returns a relation describing how a compares to b. func CompareVersions(a, b string) Relation { arel, apre := versionParts(a) brel, bpre := versionParts(b) diff --git a/internal/upgrade/upgrade_supported.go b/internal/upgrade/upgrade_supported.go index c23d63675..5dc6371b4 100644 --- a/internal/upgrade/upgrade_supported.go +++ b/internal/upgrade/upgrade_supported.go @@ -27,7 +27,8 @@ import ( "strings" ) -// Returns the latest releases, including prereleases or not depending on the argument +// LatestGithubReleases returns the latest releases, including prereleases or +// not depending on the argument func LatestGithubReleases(version string) ([]Release, error) { resp, err := http.Get("https://api.github.com/repos/syncthing/syncthing/releases?per_page=30") if err != nil { diff --git a/internal/upnp/upnp.go b/internal/upnp/upnp.go index e3e3f4742..d64ed47bf 100644 --- a/internal/upnp/upnp.go +++ b/internal/upnp/upnp.go @@ -27,7 +27,7 @@ import ( "github.com/syncthing/syncthing/internal/sync" ) -// A container for relevant properties of a UPnP InternetGatewayDevice. +// An IGD is a UPnP InternetGatewayDevice. type IGD struct { uuid string friendlyName string @@ -36,27 +36,25 @@ type IGD struct { localIPAddress string } -// The InternetGatewayDevice's UUID. func (n *IGD) UUID() string { return n.uuid } -// The InternetGatewayDevice's friendly name. func (n *IGD) FriendlyName() string { return n.friendlyName } -// The InternetGatewayDevice's friendly identifier (friendly name + IP address). +// FriendlyIdentifier returns a friendly identifier (friendly name + IP +// address) for the IGD. func (n *IGD) FriendlyIdentifier() string { return "'" + n.FriendlyName() + "' (" + strings.Split(n.URL().Host, ":")[0] + ")" } -// The URL of the InternetGatewayDevice's root device description. func (n *IGD) URL() *url.URL { return n.url } -// A container for relevant properties of a UPnP service of an IGD. +// An IGDService is a specific service provided by an IGD. type IGDService struct { serviceID string serviceURL string @@ -244,7 +242,7 @@ func parseResponse(deviceType string, resp []byte) (IGD, error) { deviceDescriptionLocation := response.Header.Get("Location") if deviceDescriptionLocation == "" { - return IGD{}, errors.New("invalid IGD response: no location specified.") + return IGD{}, errors.New("invalid IGD response: no location specified") } deviceDescriptionURL, err := url.Parse(deviceDescriptionLocation) @@ -255,7 +253,7 @@ func parseResponse(deviceType string, resp []byte) (IGD, error) { deviceUSN := response.Header.Get("USN") if deviceUSN == "" { - return IGD{}, errors.New("invalid IGD response: USN not specified.") + return IGD{}, errors.New("invalid IGD response: USN not specified") } deviceUUID := strings.TrimLeft(strings.Split(deviceUSN, "::")[0], "uuid:") @@ -361,9 +359,8 @@ func getServiceDescriptions(rootURL string, device upnpDevice) ([]IGDService, er if len(result) < 1 { return result, errors.New("[" + rootURL + "] Malformed device description: no compatible service descriptions found.") - } else { - return result, nil } + return result, nil } func getIGDServices(rootURL string, device upnpDevice, wanDeviceURN string, wanConnectionURN string, serviceURNs []string) []IGDService { @@ -488,9 +485,11 @@ func soapRequest(url, service, function, message string) ([]byte, error) { return resp, nil } -// Add a port mapping to all relevant services on the specified InternetGatewayDevice. -// Port mapping will fail and return an error if action is fails for _any_ of the relevant services. -// For this reason, it is generally better to configure port mapping for each individual service instead. +// AddPortMapping adds a port mapping to all relevant services on the +// specified InternetGatewayDevice. Port mapping will fail and return an error +// if action is fails for _any_ of the relevant services. For this reason, it +// is generally better to configure port mapping for each individual service +// instead. func (n *IGD) AddPortMapping(protocol Protocol, externalPort, internalPort int, description string, timeout int) error { for _, service := range n.services { err := service.AddPortMapping(n.localIPAddress, protocol, externalPort, internalPort, description, timeout) @@ -501,9 +500,11 @@ func (n *IGD) AddPortMapping(protocol Protocol, externalPort, internalPort int, return nil } -// Delete a port mapping from all relevant services on the specified InternetGatewayDevice. -// Port mapping will fail and return an error if action is fails for _any_ of the relevant services. -// For this reason, it is generally better to configure port mapping for each individual service instead. +// DeletePortMapping deletes a port mapping from all relevant services on the +// specified InternetGatewayDevice. Port mapping will fail and return an error +// if action is fails for _any_ of the relevant services. For this reason, it +// is generally better to configure port mapping for each individual service +// instead. func (n *IGD) DeletePortMapping(protocol Protocol, externalPort int) error { for _, service := range n.services { err := service.DeletePortMapping(protocol, externalPort) @@ -528,7 +529,7 @@ type getExternalIPAddressResponse struct { NewExternalIPAddress string `xml:"NewExternalIPAddress"` } -// Add a port mapping to the specified IGD service. +// AddPortMapping adds a port mapping to the specified IGD service. func (s *IGDService) AddPortMapping(localIPAddress string, protocol Protocol, externalPort, internalPort int, description string, timeout int) error { tpl := ` @@ -550,7 +551,7 @@ func (s *IGDService) AddPortMapping(localIPAddress string, protocol Protocol, ex return nil } -// Delete a port mapping from the specified IGD service. +// DeletePortMapping deletes a port mapping from the specified IGD service. func (s *IGDService) DeletePortMapping(protocol Protocol, externalPort int) error { tpl := ` @@ -568,8 +569,9 @@ func (s *IGDService) DeletePortMapping(protocol Protocol, externalPort int) erro return nil } -// Query the IGD service for its external IP address. -// Returns nil if the external IP address is invalid or undefined, along with any relevant errors +// GetExternalIPAddress queries the IGD service for its external IP address. +// Returns nil if the external IP address is invalid or undefined, along with +// any relevant errors func (s *IGDService) GetExternalIPAddress() (net.IP, error) { tpl := `` diff --git a/internal/versioner/external.go b/internal/versioner/external.go index 027137ca1..5a84fc721 100644 --- a/internal/versioner/external.go +++ b/internal/versioner/external.go @@ -21,13 +21,11 @@ func init() { Factories["external"] = NewExternal } -// The type holds our configuration type External struct { command string folderPath string } -// The constructor function takes a map of parameters and creates the type. func NewExternal(folderID, folderPath string, params map[string]string) Versioner { command := params["command"] @@ -42,8 +40,8 @@ func NewExternal(folderID, folderPath string, params map[string]string) Versione return s } -// Move away the named file to a version archive. If this function returns -// nil, the named file does not exist any more (has been archived). +// Archive moves the named file away to a version archive. If this function +// returns nil, the named file does not exist any more (has been archived). func (v External) Archive(filePath string) error { _, err := osutil.Lstat(filePath) if os.IsNotExist(err) { diff --git a/internal/versioner/simple.go b/internal/versioner/simple.go index 190d3764f..d5dc9750c 100644 --- a/internal/versioner/simple.go +++ b/internal/versioner/simple.go @@ -19,13 +19,11 @@ func init() { Factories["simple"] = NewSimple } -// The type holds our configuration type Simple struct { keep int folderPath string } -// The constructor function takes a map of parameters and creates the type. func NewSimple(folderID, folderPath string, params map[string]string) Versioner { keep, err := strconv.Atoi(params["keep"]) if err != nil { @@ -43,8 +41,8 @@ func NewSimple(folderID, folderPath string, params map[string]string) Versioner return s } -// Move away the named file to a version archive. If this function returns -// nil, the named file does not exist any more (has been archived). +// Archive moves the named file away to a version archive. If this function +// returns nil, the named file does not exist any more (has been archived). func (v Simple) Archive(filePath string) error { fileInfo, err := osutil.Lstat(filePath) if os.IsNotExist(err) { diff --git a/internal/versioner/staggered.go b/internal/versioner/staggered.go index a8bd94862..b54d03349 100644 --- a/internal/versioner/staggered.go +++ b/internal/versioner/staggered.go @@ -27,7 +27,6 @@ type Interval struct { end int64 } -// The type holds our configuration type Staggered struct { versionsPath string cleanInterval int64 @@ -62,7 +61,6 @@ func (v Staggered) renameOld() { } } -// The constructor function takes a map of parameters and creates the type. func NewStaggered(folderID, folderPath string, params map[string]string) Versioner { maxAge, err := strconv.ParseInt(params["maxAge"], 10, 0) if err != nil { @@ -271,8 +269,8 @@ func (v Staggered) expire(versions []string) { } } -// Move away the named file to a version archive. If this function returns -// nil, the named file does not exist any more (has been archived). +// Archive moves the named file away to a version archive. If this function +// returns nil, the named file does not exist any more (has been archived). func (v Staggered) Archive(filePath string) error { if debug { l.Debugln("Waiting for lock on ", v.versionsPath)