Icinga / icingadb

Icinga configuration and state database supporting multiple environments
https://icinga.com
GNU General Public License v2.0
60 stars 21 forks source link

Heartbeat.Err() will always return nil #316

Closed julianbrost closed 3 years ago

julianbrost commented 3 years ago

Most receiver functions for Heartbeat operate on a copy instead of a pointer.

Even though Heartbeat.setError() operates on a pointer: https://github.com/Icinga/icingadb/blob/cf3a13d3f50214efa90e3676076240cf15e6dea0/pkg/icingaredis/heartbeat.go#L149-L153

It is only called from within Heartbeat.controller() which itself operates on a copy: https://github.com/Icinga/icingadb/blob/cf3a13d3f50214efa90e3676076240cf15e6dea0/pkg/icingaredis/heartbeat.go#L79

So Heartbeat.Err() could only return something not nil when called on the copy within Heartbeat.controller() (there is no such call). If any external caller would call it (or Heartbeat.Close() which in turn returns Heartbeat.Err()), the return value would always be nil.

Given that neither seems to be called somewhere else in the code base, this won't cause any problems right now, but still looks broken.

lippserd commented 3 years ago

It's broken and being fixed in a PR I'm already working on.