FIX: Free exception messages and the reload file name

device-detection/dd/config_hash.go

@@ -27,7 +27,6 @@ package dd
 // #include "device-detection-cxx.h"
 import "C"
 import (
-	"fmt"
 	"math"
 	"unsafe"
 )
@@ -276,7 +275,7 @@ func (config *ConfigHash) SizeManagerFromFile(
 
 	// Check if no exception was thrown
 	if !exp.IsOkay() {
-		return 0, fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return 0, messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 	return uint64(s), nil
 }

device-detection/dd/exception.go

@@ -22,10 +22,18 @@
 
 package dd
 
-// #include <string.h>
-// #include "device-detection-cxx.h"
-// #include "device-detection-cxx.h"
+/*
+#include <string.h>
+#include "device-detection-cxx.h"
+
+// Frees a message the library allocated for the caller. fiftyoneDegreesFree
+// is a function pointer, which Go cannot call directly.
+static void freeMessage(const char *message) {
+	fiftyoneDegreesFree((void *)message);
+}
+*/
 import "C"
+import "errors"
 
 // Exception wraps around a pointer to a value of C Exception structure
 type Exception struct {
@@ -53,3 +61,12 @@ func (e *Exception) IsOkay() bool {
 	return (e.CPtr == nil ||
 		e.CPtr.status == C.FIFTYONE_DEGREES_STATUS_NOT_SET)
 }
+
+// messageError returns an error holding a message that the C library
+// allocated for the caller, such as the result of ExceptionGetMessage or
+// StatusGetMessage, and frees the message. errors.New keeps the text as it
+// is, where fmt.Errorf would read any % in it as a formatting verb.
+func messageError(message *C.char) error {
+	defer C.freeMessage(message)
+	return errors.New(C.GoString(message))
+}

device-detection/dd/exception_test.go

@@ -0,0 +1,80 @@
+/* *********************************************************************
+ * This Original Work is copyright of 51 Degrees Mobile Experts Limited.
+ * Copyright 2019 51 Degrees Mobile Experts Limited, 5 Charlotte Close,
+ * Caversham, Reading, Berkshire, United Kingdom RG4 7BY.
+ *
+ * This Original Work is licensed under the European Union Public Licence (EUPL)
+ * v.1.2 and is subject to its terms as set out below.
+ *
+ * If a copy of the EUPL was not distributed with this file, You can obtain
+ * one at https://opensource.org/licenses/EUPL-1.2.
+ *
+ * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be
+ * amended by the European Commission) shall be deemed incompatible for
+ * the purposes of the Work and the provisions of the compatibility
+ * clause in Article 5 of the EUPL shall not apply.
+ *
+ * If using the Work as, or as part of, a network application, by
+ * including the attribution notice(s) required under Article 5 of the EUPL
+ * in the end user terms of the application under an appropriate heading,
+ * such notice(s) shall fulfill the requirements of that article.
+ * ********************************************************************* */
+
+package dd
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestMessageError checks that a message the C library allocates reaches the
+// error unchanged. The C message for a missing data file holds the file's
+// path, and the % in the path must not be read as a formatting verb.
+func TestMessageError(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "missing-%d.hash")
+
+	manager := NewResourceManager()
+	defer manager.Free()
+
+	err := InitManagerFromFile(manager, *NewConfigHash(InMemory), "", path)
+	if err == nil {
+		t.Fatal("InitManagerFromFile() accepted a missing file")
+	}
+	if !strings.Contains(err.Error(), path) {
+		t.Errorf("error does not hold the path %q: %v", path, err)
+	}
+}
+
+// TestMessageFreed checks that a failed initialisation leaves nothing the
+// library allocated, its message included. A missing file fails in the file
+// pool with an exception for a file based profile, and with a status code
+// for the in memory profile.
+func TestMessageFreed(t *testing.T) {
+	tests := []struct {
+		name    string
+		profile PerformanceProfile
+	}{
+		{"exception", LowMemory},
+		{"status", InMemory},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			path := filepath.Join(t.TempDir(), "missing.hash")
+			manager := NewResourceManager()
+			defer manager.Free()
+			config := NewConfigHash(tt.profile)
+
+			var err error
+			left := allocatedDuring(func() {
+				err = InitManagerFromFile(manager, *config, "", path)
+			})
+			if err == nil {
+				t.Fatal("InitManagerFromFile() accepted a missing file")
+			}
+			if left != 0 {
+				t.Errorf("%d bytes were not freed after: %v", left, err)
+			}
+		})
+	}
+}

device-detection/dd/memory.go

@@ -0,0 +1,37 @@
+/* *********************************************************************
+ * This Original Work is copyright of 51 Degrees Mobile Experts Limited.
+ * Copyright 2019 51 Degrees Mobile Experts Limited, 5 Charlotte Close,
+ * Caversham, Reading, Berkshire, United Kingdom RG4 7BY.
+ *
+ * This Original Work is licensed under the European Union Public Licence (EUPL)
+ * v.1.2 and is subject to its terms as set out below.
+ *
+ * If a copy of the EUPL was not distributed with this file, You can obtain
+ * one at https://opensource.org/licenses/EUPL-1.2.
+ *
+ * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be
+ * amended by the European Commission) shall be deemed incompatible for
+ * the purposes of the Work and the provisions of the compatibility
+ * clause in Article 5 of the EUPL shall not apply.
+ *
+ * If using the Work as, or as part of, a network application, by
+ * including the attribution notice(s) required under Article 5 of the EUPL
+ * in the end user terms of the application under an appropriate heading,
+ * such notice(s) shall fulfill the requirements of that article.
+ * ********************************************************************* */
+
+package dd
+
+// #include "device-detection-cxx.h"
+import "C"
+
+// allocatedDuring runs f with the C library's memory tracking switched on and
+// returns the bytes the library allocated during f that are still allocated.
+// Tests use it to check that calls release what the library allocates. f must
+// only free library memory allocated inside f, because the tracker knows
+// nothing of earlier allocations.
+func allocatedDuring(f func()) uint64 {
+	C.fiftyoneDegreesSetUpMemoryTracking()
+	f()
+	return uint64(C.fiftyoneDegreesUnsetMemoryTracking())
+}

device-detection/dd/resource_hash.go

@@ -60,14 +60,12 @@ func InitManagerFromFile(
 
 	// Check exception
 	if !e.IsOkay() {
-		return fmt.Errorf(C.GoString(C.ExceptionGetMessage(e.CPtr)))
+		return messageError(C.ExceptionGetMessage(e.CPtr))
 	}
 
 	// Check status code
 	if s != C.SUCCESS {
-		cMessage := C.StatusGetMessage(s, cPath)
-		defer C.MemoryStandardFree(unsafe.Pointer(cMessage))
-		return fmt.Errorf(C.GoString(cMessage))
+		return messageError(C.StatusGetMessage(s, cPath))
 	}
 
 	// Construct the Set Headers list

device-detection/dd/results_hash.go

@@ -130,7 +130,7 @@ func (results *ResultsHash) DeviceId() (id string, err error) {
 
 	// Check exception
 	if !exp.IsOkay() {
-		return "", fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return "", messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 	return C.GoString(&idCStr[0]), nil
 }
@@ -155,7 +155,7 @@ func (results *ResultsHash) DeviceIdByIndex(
 
 	// Check exception
 	if !exp.IsOkay() {
-		return "", fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return "", messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 	return C.GoString(&idCStr[0]), nil
 }
@@ -307,7 +307,7 @@ func (results *ResultsHash) HasValuesByIndex(
 
 	// Check exception
 	if !exp.IsOkay() {
-		return false, fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return false, messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 	return bool(r), nil
 }
@@ -336,7 +336,7 @@ func (results *ResultsHash) NoValueReasonMessageByIndex(
 
 	// Check exception
 	if !exp.IsOkay() {
-		return "", fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return "", messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 
 	// Get no value reason message
@@ -398,8 +398,7 @@ func valuesStringWithSize(
 
 		// Check exception
 		if !exp.IsOkay() {
-			return "", fmt.Errorf(
-				C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+			return "", messageError(C.ExceptionGetMessage(exp.CPtr))
 		}
 		if ok = actualSize > defaultSize; ok {
 			// Add 1 for the null terminator
@@ -485,7 +484,7 @@ func (results *ResultsHash) MatchUserAgent(ua string) error {
 
 	// Check exception
 	if !e.IsOkay() {
-		return fmt.Errorf(C.GoString(C.ExceptionGetMessage(e.CPtr)))
+		return messageError(C.ExceptionGetMessage(e.CPtr))
 	}
 
 	return nil
@@ -501,7 +500,7 @@ func (results *ResultsHash) MatchEvidence(e *Evidence) error {
 		exp.CPtr)
 	// Check exception
 	if !exp.IsOkay() {
-		return fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 	return nil
 }
@@ -520,7 +519,7 @@ func (results *ResultsHash) MatchDeviceId(id string) error {
 
 	// Check exception
 	if !exp.IsOkay() {
-		return fmt.Errorf(C.GoString(C.ExceptionGetMessage(exp.CPtr)))
+		return messageError(C.ExceptionGetMessage(exp.CPtr))
 	}
 
 	return nil