Merge lp:~chipaca/whoopsie/failure-flags into lp:whoopsie

[John Lenton]

Fixes lp:1311571.
Added failure flags for easier testing.

Added a couple of failure flags to improve testing.

Fixed a bug (lp:1311571) while in there.

[Evan (ev)]

Looks good!

review: Approve

=== modified file 'src/identifier.c'
--- src/identifier.c	2013-08-14 08:52:35 +0000
+++ src/identifier.c	2014-05-12 13:58:41 +0000
@@ -11,6 +11,9 @@
 
 #include "identifier.h"
 
+static gint fail_next_get_mac = 0;
+static gint fail_next_get_uuid = 0;
+
 void
 whoopsie_hex_to_char (char* buf, const char *str, int len)
 {
@@ -41,6 +44,12 @@
 
     g_return_if_fail (res);
 
+    if (g_atomic_int_compare_and_exchange (&fail_next_get_mac, WHOOPSIE_FAIL_GENERIC, 0)) {
+        g_set_error (error, g_quark_from_static_string ("whoopsie-quark"), WHOOPSIE_FAILED_BY_REQUEST,
+                     "Failed by tester request");
+        return;
+    }
+
     sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_IP);
     if (sock == -1) {
         g_set_error (error, g_quark_from_static_string ("whoopsie-quark"), 0,
@@ -59,13 +68,15 @@
 
     it = ifc.ifc_req;
 
-    for (i = ifc.ifc_len / sizeof(struct ifreq); --i >= 0; it++) {
-        strcpy(ifr.ifr_name, it->ifr_name);
-        if (ioctl(sock, SIOCGIFFLAGS, &ifr) == 0) {
-            if (! (ifr.ifr_flags & IFF_LOOPBACK)) { // don't count loopback
-                if (ioctl(sock, SIOCGIFHWADDR, &ifr) == 0) {
-                    success = TRUE;
-                    break;
+    if (!g_atomic_int_compare_and_exchange (&fail_next_get_mac, WHOOPSIE_FAIL_MAC_SIOCGIFFLAGS, 0)) {
+        for (i = ifc.ifc_len / sizeof(struct ifreq); --i >= 0; it++) {
+            strcpy(ifr.ifr_name, it->ifr_name);
+            if (ioctl(sock, SIOCGIFFLAGS, &ifr) == 0) {
+                if (! (ifr.ifr_flags & IFF_LOOPBACK)) { // don't count loopback
+                    if (ioctl(sock, SIOCGIFHWADDR, &ifr) == 0) {
+                        success = TRUE;
+                        break;
+                    }
                 }
             }
         }
@@ -73,8 +84,11 @@
 
     close (sock);
 
-    if (!success)
+    if (!success) {
+        g_set_error (error, g_quark_from_static_string ("whoopsie-quark"), 0,
+                     "Unable to find a hardware address");
         return;
+    }
 
     *res = g_malloc (18);
     sprintf (*res, "%02x:%02x:%02x:%02x:%02x:%02x",
@@ -93,6 +107,12 @@
 
     g_return_if_fail (res);
 
+    if (g_atomic_int_compare_and_exchange (&fail_next_get_uuid, WHOOPSIE_FAIL_GENERIC, 0)) {
+        g_set_error (error, g_quark_from_static_string ("whoopsie-quark"), WHOOPSIE_FAILED_BY_REQUEST,
+                     "Failed by tester request");
+        return;
+    }
+
     fp = open ("/sys/class/dmi/id/product_uuid", O_RDONLY);
     if (fp < 0) {
         g_set_error (error, g_quark_from_static_string ("whoopsie-quark"), 0,
@@ -176,3 +196,12 @@
     g_free (identifier);
 }
 
+void whoopsie_identifier_fail_next_get_mac (gint how)
+{
+    g_atomic_int_set (&fail_next_get_mac, how);
+}
+
+void whoopsie_identifier_fail_next_get_uuid ()
+{
+    g_atomic_int_set (&fail_next_get_uuid, 1);
+}

=== modified file 'src/identifier.h'
--- src/identifier.h	2012-05-26 18:27:50 +0000
+++ src/identifier.h	2014-05-12 13:58:41 +0000
@@ -4,3 +4,10 @@
 void whoopsie_identifier_sha512 (char* source, char* res, GError** error);
 void whoopsie_hex_to_char (char* buf, const char *str, int len);
 #define HASHLEN 128
+
+/* for testing: */
+void whoopsie_identifier_fail_next_get_mac (gint how) __attribute__ ((__warning__ ("Only for testing")));
+void whoopsie_identifier_fail_next_get_uuid () __attribute__ ((__warning__ ("Only for testing")));
+#define WHOOPSIE_FAIL_GENERIC 1
+#define WHOOPSIE_FAIL_MAC_SIOCGIFFLAGS 2
+#define WHOOPSIE_FAILED_BY_REQUEST 42

=== modified file 'src/tests/test_identifier.c'
--- src/tests/test_identifier.c	2013-08-16 09:44:43 +0000
+++ src/tests/test_identifier.c	2014-05-12 13:58:41 +0000
@@ -28,6 +28,12 @@
 
 #include "../src/identifier.h"
 
+static gboolean
+is_root (void)
+{
+    return !(getuid () || g_getenv ("FAKEROOTKEY"));
+}
+
 static void
 test_hex_to_char (void)
 {
@@ -100,13 +106,53 @@
     
     g_free (res);
 }
+
+static void
+test_get_mac_address_fail_when_told_to (void)
+{
+    char* res = NULL;
+    GError* error = NULL;
+
+    /* check it works before */
+    whoopsie_identifier_get_mac_address (&res, &error);
+    g_assert (res != NULL);
+    g_assert (error == NULL);
+    g_free (res);
+    res = NULL;
+
+    /* set the flag */
+    whoopsie_identifier_fail_next_get_mac (WHOOPSIE_FAIL_GENERIC);
+    whoopsie_identifier_get_mac_address (&res, &error);
+    g_assert (res == NULL);
+    g_assert (error != NULL);
+    g_assert_error (error, g_quark_from_static_string ("whoopsie-quark"), WHOOPSIE_FAILED_BY_REQUEST);
+    error = NULL;
+
+    /* flag is cleared */
+    whoopsie_identifier_get_mac_address (&res, &error);
+    g_assert (res != NULL);
+    g_assert (error == NULL);
+    g_free (res);
+}
+
+static void
+test_get_mac_address_fail_when_no_siocgifflags (void)
+{
+    char* res = NULL;
+    GError* error = NULL;
+    whoopsie_identifier_fail_next_get_mac (WHOOPSIE_FAIL_MAC_SIOCGIFFLAGS);
+    whoopsie_identifier_get_mac_address (&res, &error);
+    g_assert (error != NULL);
+    g_assert (res == NULL);
+}
+
 static void
 test_get_system_uuid (void)
 {
     /* DEADBEEF-1234-1234-1234-DEADBEEF1234 */
     char* res = NULL;
     whoopsie_identifier_get_system_uuid (&res, NULL);
-    if (getuid () != 0) {
+    if (!is_root ()) {
         g_print ("Need root to run this complete test: ");
         goto out;
     }
@@ -134,6 +180,40 @@
 }
 
 static void
+test_get_system_uuid_fail_when_told_to (void)
+{
+    char* res = NULL;
+    GError* error = NULL;
+
+    if (!is_root ()) {
+        g_print ("Need root to run this complete test: ");
+    } else {
+        /* check it works before */
+        whoopsie_identifier_get_system_uuid (&res, &error);
+        g_assert (res != NULL);
+        g_assert (error == NULL);
+        g_free (res);
+        res = NULL;
+    }
+
+    /* set the flag */
+    whoopsie_identifier_fail_next_get_uuid ();
+    whoopsie_identifier_get_system_uuid (&res, &error);
+    g_assert (res == NULL);
+    g_assert (error != NULL);
+    g_assert_error (error, g_quark_from_static_string ("whoopsie-quark"), WHOOPSIE_FAILED_BY_REQUEST);
+    error = NULL;
+
+    if (is_root ()) {
+        /* flag is cleared */
+        whoopsie_identifier_get_system_uuid (&res, &error);
+        g_assert (res != NULL);
+        g_assert (error == NULL);
+        g_free (res);
+    }
+}
+
+static void
 test_sha512 (void)
 {
     char res[HASHLEN + 1] = {0};
@@ -154,7 +234,7 @@
     char hashed[HASHLEN + 1];
 
     whoopsie_identifier_generate (&result, NULL);
-    if (getuid () != 0) {
+    if (!is_root ()) {
         whoopsie_identifier_get_mac_address (&expected, NULL);
         if (!expected)
             whoopsie_identifier_get_system_uuid (&expected, NULL);
@@ -168,6 +248,45 @@
     g_free (result);
     g_free (expected);
 }
+
+static void
+test_generate_fails_on_complete_failure (void)
+{
+    char* res = NULL;
+    GError* error = NULL;
+
+    /* check we succeed on uuid failure */
+    whoopsie_identifier_fail_next_get_uuid ();
+    whoopsie_identifier_generate (&res, &error);
+    g_assert (res != NULL);
+    g_assert (error == NULL);
+
+    g_free (res);
+    res = NULL;
+
+    if (g_file_test ("/sys/class/dmi/id/product_uuid", G_FILE_TEST_EXISTS)) {
+        /* the system has a UUID */ 
+        if (!is_root ()) {
+            g_print ("Need to be root to run this complete test: ");
+        } else {
+            /* test we succeed if we can't get a mac */
+            whoopsie_identifier_fail_next_get_mac (WHOOPSIE_FAIL_GENERIC);
+            whoopsie_identifier_generate (&res, &error);
+            g_assert (res != NULL);
+            g_assert (error == NULL);
+            g_free (res);
+            res = NULL;
+        }
+    } else {
+        g_print ("(skipping UUID test on this system) ");
+    }
+
+    whoopsie_identifier_fail_next_get_mac (WHOOPSIE_FAIL_GENERIC);
+    whoopsie_identifier_fail_next_get_uuid ();
+    whoopsie_identifier_generate (&res, &error);
+    g_assert (res == NULL);
+    g_assert (error != NULL);
+}
 int
 main (int argc, char** argv)
 {
@@ -177,13 +296,15 @@
 #endif
     g_test_init (&argc, &argv, NULL);
 
-    /* This wont work when running under fakeroot. */
-    if (!g_getenv ("FAKEROOTKEY"))
-        g_test_add_func ("/whoopsie/get-system-uuid", test_get_system_uuid);
+    g_test_add_func ("/whoopsie/get-system-uuid", test_get_system_uuid);
+    g_test_add_func ("/whoopsie/get-uuid-fails-when-told", test_get_system_uuid_fail_when_told_to);
     g_test_add_func ("/whoopsie/hex-to-char", test_hex_to_char);
     g_test_add_func ("/whoopsie/get-mac-address", test_get_mac_address);
+    g_test_add_func ("/whoopsie/get-mac-fails-when-told", test_get_mac_address_fail_when_told_to);
+    g_test_add_func ("/whoopsie/get-mac-fails-when-no-siocgifflags", test_get_mac_address_fail_when_no_siocgifflags);
     g_test_add_func ("/whoopsie/sha512", test_sha512);
     g_test_add_func ("/whoopsie/identifier-generate", test_identifier_generate);
+    g_test_add_func ("/whoopsie/identifier-generate-fails-on-failure", test_generate_fails_on_complete_failure);
 
     /* Do not consider warnings to be fatal. */
     g_log_set_always_fatal (G_LOG_FATAL_MASK);