coreos / fleet

fleet ties together systemd and etcd into a distributed init system
Apache License 2.0
2.42k stars 303 forks source link

registry,fleetctl: fix bugs regarding shadowed error variables #1685

Closed dongsupark closed 7 years ago

dongsupark commented 7 years ago

: When marshal() returns a non-nil error 'err', which shadows the pre-defined 'err', we should explicitly return the second error returned from marshal(). Otherwise it will always return nil. To fix other potential errors, let's just return err explicitly. This was discovered by running "go tool vet":

$ go tool vet --shadow ./registry
registry/job.go:316: declaration of "err" shadows declaration at registry/job.go:315

: In matchLocalFileAndUnit(), if os.Stat() returns nil and getUnitFromFile() returns non-nil error, then in the end matchLocalFileAndUnit() will return (false, nil). That's not the expected result. It should have returned the second error that came from getUnitFromFile(). To avoid further potential bugs, let's follow the idiomatic coding style: first check "err != nil", and fail-fast. This issue was discovered by running "go tool vet".

tixxdz commented 7 years ago

lgtm thanks @dongsupark , don't know why or if golang is supposed to warn about a local variable that shadows another local variable.

dongsupark commented 7 years ago

@tixxdz Merged. Thanks for the review!