From 9f6ee5f3598f15c4a2911f24cdae0bcd86e6388b Mon Sep 17 00:00:00 2001
From: Matthew Kennedy <matthewkennedy@outlook.com>
Date: Wed, 27 Jul 2022 02:41:21 -0700
Subject: [PATCH] detect bad Lua sensor name when registering (#4391)

* format

* lua error on bad sensor

* comment

* name validation

* soft check before the hard check causes a fatal error

* s
---
 firmware/controllers/lua/lua_hooks.cpp   | 51 +++++++++++++++++++++---
 firmware/controllers/sensors/sensor.cpp  | 17 ++++----
 unit_tests/tests/sensor/basic_sensor.cpp |  1 +
 3 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/firmware/controllers/lua/lua_hooks.cpp b/firmware/controllers/lua/lua_hooks.cpp
index c1ef40ccd4..625108279c 100644
--- a/firmware/controllers/lua/lua_hooks.cpp
+++ b/firmware/controllers/lua/lua_hooks.cpp
@@ -68,9 +68,19 @@ static int lua_getSensorByIndex(lua_State* l) {
 	return getSensor(l, static_cast<SensorType>(zeroBasedSensorIndex));
 }
 
+static SensorType findSensorByName(lua_State* l, const char* name) {
+	SensorType type = findSensorTypeByName(name);
+
+	if (l && type == SensorType::Invalid) {
+		luaL_error(l, "Invalid sensor type: %s", name);
+	}
+
+	return type;
+}
+
 static int lua_getSensorByName(lua_State* l) {
 	auto sensorName = luaL_checklstring(l, 1, nullptr);
-	SensorType type = findSensorTypeByName(sensorName);
+	SensorType type = findSensorByName(l, sensorName);
 
 	return getSensor(l, type);
 }
@@ -340,17 +350,46 @@ static int lua_setAirmass(lua_State* l) {
 
 #endif // EFI_UNIT_TEST
 
+// TODO: PR this back in to https://github.com/gengyong/luaaa
+namespace LUAAA_NS {
+    template<typename TCLASS, typename ...ARGS>
+    struct PlacementConstructorCaller<TCLASS, lua_State*, ARGS...>
+    {
+        // this speciailization passes the Lua state to the constructor as first argument, as it shouldn't
+        // participate in the index generation as it's not a normal parameter passed via the Lua stack.
+
+        static TCLASS * Invoke(lua_State * state, void * mem)
+        {
+            return InvokeImpl(state, mem, typename make_indices<sizeof...(ARGS)>::type());
+        }
+
+    private:
+        template<std::size_t ...Ns>
+        static TCLASS * InvokeImpl(lua_State * state, void * mem, indices<Ns...>)
+        {
+            (void)state;
+            return new(mem) TCLASS(state, LuaStack<ARGS>::get(state, Ns + 1)...);
+        }
+    };
+}
+
 struct LuaSensor final : public StoredValueSensor {
-	LuaSensor() : LuaSensor("Invalid") { }
+	LuaSensor() : LuaSensor(nullptr, "Invalid") { }
 
 	~LuaSensor() {
 		unregister();
 	}
 
-	LuaSensor(const char* name)
-		: StoredValueSensor(findSensorTypeByName(name), MS2NT(100))
+	LuaSensor(lua_State* l, const char* name)
+		: StoredValueSensor(findSensorByName(l, name), MS2NT(100))
 	{
-		Register();
+		// do a soft collision check to avoid a fatal error from the hard check in Register()
+		if (l && Sensor::hasSensor(type())) {
+			luaL_error(l, "Tried to create a Lua sensor of type %s, but one was already registered.", getSensorName());
+		} else {
+			Register();
+			efiPrintf("LUA registered sensor of type %s", getSensorName());
+		}
 	}
 
 	void set(float value) {
@@ -418,7 +457,7 @@ void configureRusefiLuaHooks(lua_State* l) {
 
 	LuaClass<LuaSensor> luaSensor(l, "Sensor");
 	luaSensor
-		.ctor<const char*>()
+		.ctor<lua_State*, const char*>()
 		.fun("set", &LuaSensor::set)
 		.fun("invalidate", &LuaSensor::invalidate);
 
diff --git a/firmware/controllers/sensors/sensor.cpp b/firmware/controllers/sensors/sensor.cpp
index e88fc7e366..305e0eba3b 100644
--- a/firmware/controllers/sensors/sensor.cpp
+++ b/firmware/controllers/sensors/sensor.cpp
@@ -245,12 +245,13 @@ void Sensor::unregister() {
  * todo: some sort of hashmap in the future?
  */
 SensorType findSensorTypeByName(const char *name) {
-    for (int i = 0;i<(int)SensorType::PlaceholderLast;i++) {
-    	SensorType type = (SensorType)i;
-    	const char *sensorName = getSensorType(type);
-    	if (strEqualCaseInsensitive(sensorName, name)) {
-    		return type;
-    	}
-    }
-    return SensorType::Invalid;
+	for (int i = 0;i<(int)SensorType::PlaceholderLast;i++) {
+		SensorType type = (SensorType)i;
+		const char *sensorName = getSensorType(type);
+		if (strEqualCaseInsensitive(sensorName, name)) {
+			return type;
+		}
+	}
+
+	return SensorType::Invalid;
 }
diff --git a/unit_tests/tests/sensor/basic_sensor.cpp b/unit_tests/tests/sensor/basic_sensor.cpp
index 7f9d6a8e37..0a49704004 100644
--- a/unit_tests/tests/sensor/basic_sensor.cpp
+++ b/unit_tests/tests/sensor/basic_sensor.cpp
@@ -110,4 +110,5 @@ TEST_F(SensorBasic, HasSensorMock) {
 
 TEST_F(SensorBasic, FindByName) {
 	ASSERT_EQ(SensorType::Clt, findSensorTypeByName("Clt"));
+	ASSERT_EQ(SensorType::Clt, findSensorTypeByName("cLT"));
 }