Fixes for probable bugs reported by Qodana

apricot/local/local_test.go

@@ -31,11 +31,21 @@ var _ = BeforeSuite(func() {
 	for _, configFile := range configFiles {
 		from, err := os.Open("./" + configFile)
 		Expect(err).NotTo(HaveOccurred())
-		defer from.Close()
+		defer func() {
+			err := from.Close()
+			if err != nil {
+				Expect(err).NotTo(HaveOccurred())
+			}
+		}()
 
 		to, err := os.OpenFile(*tmpDir+"/"+configFile, os.O_RDWR|os.O_CREATE, 0666)
 		Expect(err).NotTo(HaveOccurred())
-		defer to.Close()
+		defer func() {
+			err := to.Close()
+			if err != nil {
+				Expect(err).NotTo(HaveOccurred())
+			}
+		}()
 
 		_, err = io.Copy(to, from)
 		Expect(err).NotTo(HaveOccurred())
@@ -45,5 +55,6 @@ var _ = BeforeSuite(func() {
 })
 
 var _ = AfterSuite(func() {
-	os.RemoveAll(*tmpDir)
+	err := os.RemoveAll(*tmpDir)
+	Expect(err).NotTo(HaveOccurred())
 })

apricot/local/service.go

@@ -508,7 +508,10 @@ func (s *Service) GetCRUCardsForHost(hostname string) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	json.Unmarshal([]byte(cfgCards), &cards)
+	err = json.Unmarshal([]byte(cfgCards), &cards)
+	if err != nil {
+		return nil, err
+	}
 	unique := make(map[string]bool)
 	for _, card := range cards {
 		if _, value := unique[card.Serial]; !value {

apricot/local/service_test.go

@@ -461,6 +461,12 @@ var _ = Describe("local service", func() {
 					Expect(err).To(HaveOccurred())
 				})
 			})
+			When("retrieving the CRU cards for an FLP with invalid \"cards\" JSON", func() {
+				It("should produce an error", func() {
+					cards, err = svc.GetCRUCardsForHost("flp500")
+					Expect(err).To(HaveOccurred())
+				})
+			})
 		})
 		Describe("getting endpoints for a CRU card", func() {
 			var (

apricot/local/service_test.yaml

@@ -192,4 +192,6 @@ o2:
       flp003:
         cards: "{ \"key\" : \"value\" }"
       flp100:
-        cards: "{ \"key\" : \"value\" }"
+        cards: "{ \"key\" : \"value\" }"
+      flp500:
+        cards: "invalid json{"

apricot/local/servicehttp.go

@@ -615,21 +615,42 @@ func (httpsvc *HttpService) ApiPrintClusterInformation(w http.ResponseWriter, r
 				log.WithError(err).Warn("Error, could not marshal inventory.")
 			}
 		}
-		fmt.Fprintln(w, string(result))
+		_, err = fmt.Fprintln(w, string(result))
+		if err != nil {
+			log.WithField("result", string(result)).
+				WithField(infologger.Level, infologger.IL_Support).
+				WithError(err).
+				Warn("Error, could not write a part of HTTP response to response writer.")
+		}
 	case "text":
 		fallthrough
 	default:
 		w.Header().Set("Content-Type", "text/plain")
 		w.WriteHeader(http.StatusOK)
 		if hosts != nil {
 			for _, hostname := range hosts {
-				fmt.Fprintf(w, "%s\n", hostname)
+				_, err := fmt.Fprintf(w, "%s\n", hostname)
+				if err != nil {
+					log.WithField(infologger.Level, infologger.IL_Support).
+						WithError(err).
+						Warn("Error, could not write a part of HTTP response to response writer.")
+				}
 			}
 		} else if inventory != nil {
 			for detector, flps := range inventory {
-				fmt.Fprintf(w, "%s\n", detector)
+				_, err := fmt.Fprintf(w, "%s\n", detector)
+				if err != nil {
+					log.WithField(infologger.Level, infologger.IL_Support).
+						WithError(err).
+						Warn("Error, could not write a part of HTTP response to response writer.")
+				}
 				for _, hostname := range flps {
-					fmt.Fprintf(w, "\t%s\n", hostname)
+					_, err = fmt.Fprintf(w, "\t%s\n", hostname)
+					if err != nil {
+						log.WithField(infologger.Level, infologger.IL_Support).
+							WithError(err).
+							Warn("Error, could not write a part of HTTP response to response writer.")
+					}
 				}
 			}
 		}

common/event/writer.go

@@ -133,7 +133,11 @@ func (w *KafkaWriter) Close() {
 		w.runningWorkers.Add(2)
 		close(w.toBatchMessagesChan)
 		w.runningWorkers.Wait()
-		w.Writer.Close()
+		err := w.Writer.Close()
+		if err != nil {
+			log.WithField(infologger.Level, infologger.IL_Devel).
+				Errorf("failed to close writer: %v", err)
+		}
 	}
 }
 

common/event/writer_test.go

@@ -49,7 +49,8 @@ var _ = Describe("Writer", func() {
 			writer.writeFunction = func(messages []kafka.Message) {
 				Expect(len(messages)).To(Equal(1))
 				event := &pb.Event{}
-				proto.Unmarshal(messages[0].Value, event)
+				err := proto.Unmarshal(messages[0].Value, event)
+				Expect(err).To(BeNil())
 				Expect(event.GetCoreStartEvent().FrameworkId).To(Equal("FrameworkId"))
 				channel <- struct{}{}
 			}

common/gera/map_test.go

@@ -250,18 +250,18 @@ pdp_workflow_parameters: ""
 		})
 
 		It("should correctly return a copy of the whole structure", func() {
-			copy := stringMap.Copy()
-			Expect(copy).NotTo(BeNil())
-			Expect(copy == stringMap).NotTo(BeTrue())
-			Expect(copy).To(Equal(stringMap))
-			Expect(copy.Raw()).To(Equal(stringMap.Raw()))
-			Expect(copy.Len()).To(Equal(stringMap.Len()))
-			copy.Set("detector", "test")
-			Expect(copy).NotTo(Equal(stringMap))
-			Expect(copy.Raw()).NotTo(Equal(stringMap.Raw()))
-			Expect(copy.Len()).To(Equal(stringMap.Len()))
-			copy.Set("detector2", "test2")
-			Expect(copy.Len()).To(Equal(stringMap.Len() + 1))
+			mapCopy := stringMap.Copy()
+			Expect(mapCopy).NotTo(BeNil())
+			Expect(mapCopy == stringMap).NotTo(BeTrue())
+			Expect(mapCopy).To(Equal(stringMap))
+			Expect(mapCopy.Raw()).To(Equal(stringMap.Raw()))
+			Expect(mapCopy.Len()).To(Equal(stringMap.Len()))
+			mapCopy.Set("detector", "test")
+			Expect(mapCopy).NotTo(Equal(stringMap))
+			Expect(mapCopy.Raw()).NotTo(Equal(stringMap.Raw()))
+			Expect(mapCopy.Len()).To(Equal(stringMap.Len()))
+			mapCopy.Set("detector2", "test2")
+			Expect(mapCopy.Len()).To(Equal(stringMap.Len() + 1))
 		})
 
 		It("should correctly return a copy of the underlying map", func() {

common/monitoring/common.go

@@ -35,11 +35,11 @@ type key struct {
 }
 
 func metricNameTagsToHash(hash *maphash.Hash, metric *Metric) {
-	hash.WriteString(metric.name)
+	_, _ = hash.WriteString(metric.name)
 
 	for _, tag := range metric.tags {
-		hash.WriteString(tag.name)
-		hash.WriteString(tag.value)
+		_, _ = hash.WriteString(tag.name)
+		_, _ = hash.WriteString(tag.value)
 	}
 }
 

common/monitoring/metricsreservoirsampling.go

@@ -66,7 +66,7 @@ func metricFieldToFloat64(field any) float64 {
 func (this *MetricsReservoirSampling) AddMetric(metric *Metric) {
 	for fieldName, field := range metric.fields {
 		metricNameTagsToHash(&this.hash, metric)
-		this.hash.WriteString(fieldName)
+		_, _ = this.hash.WriteString(fieldName)
 		k := key{nameTagsHash: hashValueAndReset(&this.hash), timestamp: time.Unix(metric.timestamp.Unix(), 0)}
 		if storedMetric, ok := this.metricsBuckets[k]; !ok {
 			newReservoir := newReservoirSampling(fieldName, reservoirSize)
@@ -88,16 +88,16 @@ func (this *MetricsReservoirSampling) GetMetrics() []Metric {
 	for key, reservoirMetric := range this.metricsBuckets {
 		m := Metric{name: reservoirMetric.metric.name, tags: reservoirMetric.metric.tags, timestamp: key.timestamp}
 
-		mean, median, min, p10, p30, p70, p90, max, count, poolSize := reservoirMetric.reservoir.GetStats()
+		mean, median, minimum, p10, p30, p70, p90, maximum, count, poolSize := reservoirMetric.reservoir.GetStats()
 
 		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_mean", mean)
 		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_median", median)
-		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_min", min)
+		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_min", minimum)
 		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_p10", p10)
 		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_p30", p30)
 		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_p70", p70)
 		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_p90", p90)
-		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_max", max)
+		m.SetFieldFloat64(reservoirMetric.reservoir.name+"_max", maximum)
 		m.SetFieldUInt64(reservoirMetric.reservoir.name+"_count", count)
 		m.SetFieldUInt64(reservoirMetric.reservoir.name+"_poolsize", poolSize)
 

common/monitoring/monitoring.go

@@ -120,7 +120,10 @@ func exportMetricsAndReset(w http.ResponseWriter, r *http.Request) {
 	if metricsToConvert == nil {
 		metricsToConvert = make([]Metric, 0)
 	}
-	Format(w, metricsToConvert)
+	err := Format(w, metricsToConvert)
+	if err != nil {
+		log.WithField(infologger.Level, infologger.IL_Devel).Errorf("Failed to export metrics: %v", err)
+	}
 }
 
 func Send(metric *Metric) {

common/monitoring/monitoring_test.go

@@ -382,7 +382,7 @@ func TestApproximateHistogram(t *testing.T) {
 		histo.AddPoint(10)
 	}
 
-	mean, median, min, p10, p30, p70, p90, max, count, poolSize := histo.GetStats()
+	mean, median, minimum, p10, p30, p70, p90, maximum, count, poolSize := histo.GetStats()
 
 	if count != 20 {
 		t.Errorf("wrong count before reset, expected 20, got %d", count)
@@ -392,7 +392,7 @@ func TestApproximateHistogram(t *testing.T) {
 		t.Errorf("wrong poolSize, expected 20, got %d", poolSize)
 	}
 
-	if mean != 10 || median != 10 || min != 10 || p10 != 10 || p30 != 10 || p70 != 10 || p90 != 10 || max != 10 {
+	if mean != 10 || median != 10 || minimum != 10 || p10 != 10 || p30 != 10 || p70 != 10 || p90 != 10 || maximum != 10 {
 		t.Errorf("one of the values is not 10 when it should be 10, mean %v, median %v, 10p %v, 30p %v, 70p %v, 90p %v", mean, median, p10, p30, p70, p90)
 	}
 
@@ -410,7 +410,7 @@ func TestApproximateHistogram(t *testing.T) {
 		histo.AddPoint(10)
 	}
 
-	mean, median, min, p10, p30, p70, p90, max, count, poolSize = histo.GetStats()
+	mean, median, minimum, p10, p30, p70, p90, maximum, count, poolSize = histo.GetStats()
 
 	if count != 2000 {
 		t.Errorf("wrong count before reset, expected 2000, got %d", count)
@@ -420,7 +420,7 @@ func TestApproximateHistogram(t *testing.T) {
 		t.Errorf("wrong poolSize, expected 500, got %d", poolSize)
 	}
 
-	if mean != 10 || median != 10 || min != 10 || p10 != 10 || p30 != 10 || p70 != 10 || p90 != 10 || max != 10 {
+	if mean != 10 || median != 10 || minimum != 10 || p10 != 10 || p30 != 10 || p70 != 10 || p90 != 10 || maximum != 10 {
 		t.Errorf("one of the values is not 10 when it should be 10, mean %v, median %v, 10p %v, 30p %v, 70p %v, 90p %v", mean, median, p10, p30, p70, p90)
 	}
 
@@ -438,7 +438,7 @@ func TestApproximateHistogram(t *testing.T) {
 		histo.AddPoint((float64(rand.Int63n(100))))
 	}
 
-	mean, median, min, p10, p30, p70, p90, max, count, poolSize = histo.GetStats()
+	mean, median, minimum, p10, p30, p70, p90, maximum, count, poolSize = histo.GetStats()
 
 	if count != 10000 {
 		t.Errorf("wrong count before reset, expected 10000, got %d", count)
@@ -456,8 +456,8 @@ func TestApproximateHistogram(t *testing.T) {
 		t.Errorf("wrong median value, expected 50+-10 got %v", median)
 	}
 
-	if float64(min) > 10 {
-		t.Errorf("wrong min value, expected 0+-10 got %v", min)
+	if float64(minimum) > 10 {
+		t.Errorf("wrong min value, expected 0+-10 got %v", minimum)
 	}
 
 	if math.Abs(float64(p10-10)) > 10 {
@@ -476,8 +476,8 @@ func TestApproximateHistogram(t *testing.T) {
 		t.Errorf("wrong 90p value, expected 90+-10 got %v", p90)
 	}
 
-	if math.Abs(float64(max-100)) > 10 {
-		t.Errorf("wrong max value, expected 100+-10 got %v", max)
+	if math.Abs(float64(maximum-100)) > 10 {
+		t.Errorf("wrong max value, expected 100+-10 got %v", maximum)
 	}
 }
 

configuration/cfgbackend/configuration_suite_test.go

@@ -28,16 +28,27 @@ var _ = BeforeSuite(func() {
 	// copy config file
 	from, err := os.Open("./" + configFile)
 	Expect(err).NotTo(HaveOccurred())
-	defer from.Close()
+	defer func() {
+		err := from.Close()
+		if err != nil {
+			Expect(err).NotTo(HaveOccurred())
+		}
+	}()
 
 	to, err := os.OpenFile(*tmpDir+"/"+configFile, os.O_RDWR|os.O_CREATE, 0666)
 	Expect(err).NotTo(HaveOccurred())
-	defer to.Close()
+	defer func() {
+		err := to.Close()
+		if err != nil {
+			Expect(err).NotTo(HaveOccurred())
+		}
+	}()
 
 	_, err = io.Copy(to, from)
 	Expect(err).NotTo(HaveOccurred())
 })
 
 var _ = AfterSuite(func() {
-	os.RemoveAll(*tmpDir)
+	err := os.RemoveAll(*tmpDir)
+	Expect(err).NotTo(HaveOccurred())
 })

configuration/template/fields.go

@@ -218,6 +218,7 @@ func (vs *VarStack) consolidated(stage Stage) (consolidatedStack map[string]stri
 		if err != nil {
 			return
 		}
+	case _STAGE_MAX:
 	}
 
 	consolidated := gera.MakeMapWithMap(vs.Locals).Wrap(gera.MakeMapWithMap(userVars).Wrap(gera.MakeMapWithMap(vars).Wrap(gera.MakeMapWithMap(defaults))))
@@ -342,7 +343,7 @@ func (fields Fields) Execute(confSvc ConfigurationService, parentPath string, va
 
 func copyMap(src map[string]interface{}, dest map[string]interface{}) {
 	for k, v := range src {
-		vm, ok := v.(map[string]interface{})
+		vmap, ok := v.(map[string]interface{})
 		if ok {
 			var destk map[string]interface{}
 			if _, exists := dest[k]; exists {
@@ -353,7 +354,7 @@ func copyMap(src map[string]interface{}, dest map[string]interface{}) {
 			} else {
 				destk = make(map[string]interface{})
 			}
-			copyMap(vm, destk)
+			copyMap(vmap, destk)
 			dest[k] = destk
 		} else {
 			dest[k] = v

configuration/template/template_test.go

@@ -28,17 +28,28 @@ var _ = BeforeSuite(func() {
 	for _, configFile := range configFiles {
 		from, err := os.Open("./" + configFile)
 		Expect(err).NotTo(HaveOccurred())
-		defer from.Close()
+		defer func() {
+			err := from.Close()
+			if err != nil {
+				Expect(err).NotTo(HaveOccurred())
+			}
+		}()
 
 		to, err := os.OpenFile(*tmpDir+"/"+configFile, os.O_RDWR|os.O_CREATE, 0666)
 		Expect(err).NotTo(HaveOccurred())
-		defer to.Close()
+		defer func() {
+			err := to.Close()
+			if err != nil {
+				Expect(err).NotTo(HaveOccurred())
+			}
+		}()
 
 		_, err = io.Copy(to, from)
 		Expect(err).NotTo(HaveOccurred())
 	}
 })
 
 var _ = AfterSuite(func() {
-	os.RemoveAll(*tmpDir)
+	err := os.RemoveAll(*tmpDir)
+	Expect(err).NotTo(HaveOccurred())
 })

core/environment/utils.go

@@ -28,6 +28,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"github.com/AliceO2Group/Control/common/logger/infologger"
 	"os"
 	"sort"
 
@@ -93,7 +94,10 @@ func sortMapToString(m map[string]string) string {
 	b := new(bytes.Buffer)
 
 	for _, k := range keys {
-		fmt.Fprintf(b, "%s=\"%s\"\n", k, m[k])
+		_, err := fmt.Fprintf(b, "%s=\"%s\"\n", k, m[k])
+		if err != nil {
+			log.WithField(infologger.Level, infologger.IL_Devel).Errorf("Error formatting or buffering string for key %s: %v", k, err)
+		}
 	}
 	return b.String()
 }