Replace *out function parameter with return value

Using return value instead of memory manipulation seem to be cleaner solution and less error prone
due to avoiding pointer usage.

Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>

5 files changed, 27 insertions, 30 deletions

diff --git a/NK_C_API.cc b/NK_C_API.cc
index 56340ac..d8b8168 100644
--- a/NK_C_API.cc
+++ b/NK_C_API.cc
@@ -238,28 +238,23 @@ extern "C" {
 	}
 
 
-	NK_C_API bool NK_get_device_model(enum NK_device_model *out) {
-		if (out == nullptr) {
-			return false;
-		}
+	NK_C_API enum NK_device_model NK_get_device_model() {
 		auto m = NitrokeyManager::instance();
 		try {
 			auto model = m->get_connected_device_model();
 			switch (model) {
 				case DeviceModel::PRO:
-				    *out = NK_PRO;
-				    return true;
+				    return NK_PRO;
 				case DeviceModel::STORAGE:
-				    *out = NK_STORAGE;
-				    return true;
+				    return NK_STORAGE;
 				default:
-				    /* unknown device -- should not happen */
-				    return false;
+				    /* unknown or not connected device */
+				    return NK_device_model::NK_DISCONNECTED;
 			}
 		} catch (const DeviceNotConnected& e) {
-			return false;
+			return NK_device_model::NK_DISCONNECTED;
 		}
-        }
+}
 
 
 	void clear_string(std::string &s) {
diff --git a/NK_C_API.h b/NK_C_API.h
index 10cc29f..a739aff 100644
--- a/NK_C_API.h
+++ b/NK_C_API.h
@@ -41,14 +41,18 @@ extern "C" {
          * The Nitrokey device models supported by the API.
          */
         enum NK_device_model {
+						/**
+						 * Use, if no supported device is connected
+						 */
+        		NK_DISCONNECTED = 0,
             /**
              * Nitrokey Pro.
              */
-            NK_PRO,
+            NK_PRO = 1,
             /**
              * Nitrokey Storage.
              */
-            NK_STORAGE
+            NK_STORAGE = 2
         };
 
 	/**
@@ -90,15 +94,12 @@ extern "C" {
 	NK_C_API int NK_logout();
 
 	/**
-	 * Query the model of the connected device.  If the out argument is
-         * NULL or if there is no connected device, this function returns
-         * false.  Otherwise it returns true and sets the target of the out
-	 * pointer to the model of the connected device.
+	 * Query the model of the connected device.
+	 * Returns the model of the connected device or NK_DISCONNECTED.
 	 *
-	 * @param out a pointer to write the model to
 	 * @return true if a device is connected and the out argument has been set
 	 */
-	NK_C_API bool NK_get_device_model(enum NK_device_model *out);
+	NK_C_API enum NK_device_model NK_get_device_model();
 
 	/**
 	 * Return the debug status string. Debug purposes.
diff --git a/unittest/conftest.py b/unittest/conftest.py
index 9af67ac..49f1502 100644
--- a/unittest/conftest.py
+++ b/unittest/conftest.py
@@ -44,7 +44,6 @@ def C(request=None):
     cnt = 0
     a = iter(declarations)
     for declaration in a:
-        if 'NK_device_model' in declaration: continue
         if declaration.strip().startswith('NK_C_API'):
             declaration = declaration.replace('NK_C_API', '').strip()
             while ';' not in declaration:
diff --git a/unittest/test_C_API.cpp b/unittest/test_C_API.cpp
index f38d0b6..1964738 100644
--- a/unittest/test_C_API.cpp
+++ b/unittest/test_C_API.cpp
@@ -87,18 +87,15 @@ TEST_CASE("multiple devices with ID", "[BASIC]") {
 }
 
 TEST_CASE("Get device model", "[BASIC]") {
-    auto success = NK_get_device_model(nullptr);
-    REQUIRE(!success);
-
     NK_logout();
-    NK_device_model model = static_cast<NK_device_model>(3);
-    success = NK_get_device_model(&model);
-    REQUIRE(!success);
+    NK_device_model model = NK_get_device_model();
+    REQUIRE(model == NK_device_model::NK_DISCONNECTED);
 
-    auto result = NK_login_auto();
-    REQUIRE(result != 0);
-    success = NK_get_device_model(&model);
+    auto success = NK_login_auto() == 1;
     REQUIRE(success);
+    model = NK_get_device_model();
+    REQUIRE(model != NK_device_model::NK_DISCONNECTED);
+
     REQUIRE((model == NK_PRO || model == NK_STORAGE));
     NK_logout();
 }
diff --git a/unittest/test_pro.py b/unittest/test_pro.py
index 53588f6..6fac172 100644
--- a/unittest/test_pro.py
+++ b/unittest/test_pro.py
@@ -872,3 +872,8 @@ def test_TOTP_codes_from_nitrokeyapp(secret, C):
     lib_at = lambda : bb(oath.totp(secret, period=period))
     print (lib_at())
     assert lib_at() == code_device
+
+
+def test_get_device_model(C):
+    assert C.NK_get_device_model() != 0
+    # assert C.NK_get_device_model() != C.NK_DISCONNECTED
\ No newline at end of file