security-manager-tests: properly check exit status of child processes 66/65166/3
authorRafal Krypa <r.krypa@samsung.com>
Thu, 7 Apr 2016 14:11:04 +0000 (16:11 +0200)
committerZbigniew Jasinski <z.jasinski@samsung.com>
Fri, 8 Apr 2016 10:23:20 +0000 (03:23 -0700)
Test cases 10-14, 15*, 16, 17 and 21 create one or more child processes
to run tests across. But they fail to properly verify whether the
child returned properly or not.
Although forked processes do use RUNNER_* macros, they may fail without
parent process noticing and the tests passes when it should not.

Change-Id: Ie4ba9de8f47782c800d877131412f8afdbfe2100
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
src/common/tests_common.cpp
src/common/tests_common.h
src/security-manager-tests/security_manager_tests.cpp

index 470235dfd848c3a6e00c6f658ace8db3dac13e85..52ef252c0fd97c9c2933ee1d737f4793344be30b 100644 (file)
@@ -26,6 +26,7 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/wait.h>
 #include <unistd.h>
 #include <grp.h>
 #include <errno.h>
@@ -230,3 +231,12 @@ void removeDir(const std::string &path)
 
     RUNNER_ASSERT_ERRNO_MSG(0 == rmdir(path.c_str()), "rmdir for <" << path << "> failed");
 }
+
+void waitPid(pid_t pid)
+{
+    int status;
+    pid_t ret = waitpid(pid, &status, 0);
+    RUNNER_ASSERT_MSG((ret != -1) && WIFEXITED(status) && WEXITSTATUS(status) == 0,
+        "Child process exited abnormally" <<
+        ": ret=" << ret << ", errno=" << errno << ", status=" << status);
+}
index b2d12fb85f74bcbaa8d57ede11eb685170fd5e51..6a355516b80e0fbd3f3289a4709b77ed95aa809a 100644 (file)
@@ -55,6 +55,7 @@ void mktreeSafe(const std::string &path, mode_t mode);
 void creatSafe(const std::string &path, mode_t mode);
 void symlinkSafe(const std::string &targetPath, const std::string &linkPath);
 void removeDir(const std::string &path);
+void waitPid(pid_t pid);
 
 
 #define RUNNER_TEST_SMACK(Proc, ...)                                                        \
index 4f2a6dc90727013ca4222701f8524bada746fe65..8db5a117ef63ad513b0ec0fa93bbadb74618c033 100644 (file)
@@ -1185,9 +1185,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_10_privacy_manager_fetch_whole_policy_
 
         //Start child process
         pipe.post();
-
-        int status;
-        wait(&status);
+        waitPid(pid);
 
         tmpUser.remove();
     } else { //child process
@@ -1294,9 +1292,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_11_privacy_manager_fetch_whole_policy_
 
         //Start child
         pipe.post();
-
-        int status;
-        wait(&status);
+        waitPid(pid);
 
         for (auto &user : users)
             user.remove();
@@ -1406,10 +1402,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_12_privacy_manager_fetch_whole_policy_
 
         //Start child process
         pipe.post();
-
-        //Wait for child to finish
-        int status;
-        wait(&status);
+        waitPid(pid);
 
         for (auto &user : users)
             user.remove();
@@ -1594,20 +1587,13 @@ RUNNER_MULTIPROCESS_TEST(security_manager_13_privacy_manager_fetch_policy_after_
                 //check_app_after_install(MANY_APPS[i].c_str(), MANY_APPS_PKGS[i].c_str());
             };
 
-            int status;
             //Start child #1
             sync[0].post();
-
-            //Wait until child #1 finishes
-            pid_t ret = wait(&status);
-            RUNNER_ASSERT_MSG((ret != -1) && WIFEXITED(status), "Updating privileges failed");
+            waitPid(pid[0]);
 
             //Start child #2
             sync[1].post();
-
-            //Wait until child #2 finishes
-            ret = wait(&status);
-            RUNNER_ASSERT_MSG((ret =-1) && WIFEXITED(status), "Listing privileges failed");
+            waitPid(pid[1]);
 
             for (auto &user : users)
                 user.remove();
@@ -1678,10 +1664,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_14_privacy_manager_fetch_and_update_po
 
         //Start child process
         pipe.post();
-
-        int status;
-        //Wait for child process to finish
-        wait(&status);
+        waitPid(pid);
 
         //switch back to root
         for (auto &user : users)
@@ -1799,8 +1782,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_15_privacy_manager_send_policy_update_
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid);
 
         admin.adminCheck(check_start_bucket, false, generateAppLabel(update_app_id).c_str(),
                 std::to_string(static_cast<int>(msg.uid)).c_str(), update_privilege, CYNARA_ADMIN_ALLOW, nullptr);
@@ -1868,8 +1850,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_15_privacy_manager_send_policy_update_
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid);
 
         admin.adminCheck(check_start_bucket, false, generateAppLabel(update_other_app_id).c_str(),
                 std::to_string(static_cast<int>(msg.uid)).c_str(), update_privilege, CYNARA_ADMIN_ALLOW, nullptr);
@@ -1938,8 +1919,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_15_privacy_manager_send_policy_update_
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid);
 
         admin.adminCheck(check_start_bucket, false, generateAppLabel(update_app_id).c_str(),
                 std::to_string(static_cast<int>(msg.uid)).c_str(), update_privilege, CYNARA_ADMIN_ALLOW, nullptr);
@@ -1998,8 +1978,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_16_policy_levels_get)
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid);
     }
     if(pid == 0)
     {
@@ -2057,7 +2036,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_17_privacy_manager_delete_policy_for_s
 
     int pipefd[2];
     int pipefd2[2];
-    pid_t pid;
+    pid_t pid[2];
     int result = 0;
     ScopedProcessLabel keepLabel;
 
@@ -2067,9 +2046,9 @@ RUNNER_MULTIPROCESS_TEST(security_manager_17_privacy_manager_delete_policy_for_s
     TemporaryTestUser user(username, GUM_USERTYPE_NORMAL, false);
     user.create();
 
-    pid = fork();
-    RUNNER_ASSERT_MSG(pid >= 0, "fork failed");
-    if (pid != 0)//parent process
+    pid[0] = fork();
+    RUNNER_ASSERT_MSG(pid[0] >= 0, "fork failed");
+    if (pid[0] != 0)//parent process
     {
         FdUniquePtr pipeptr(pipefd+1);
         close(pipefd[0]);
@@ -2083,14 +2062,13 @@ RUNNER_MULTIPROCESS_TEST(security_manager_17_privacy_manager_delete_policy_for_s
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid[0]);
 
         admin.adminCheck(check_start_bucket, false, generateAppLabel(update_app_id).c_str(),
                 std::to_string(static_cast<int>(msg.uid)).c_str(), update_privilege, CYNARA_ADMIN_ALLOW, nullptr);
 
-        pid = fork();
-        if (pid != 0)//parent process
+        pid[1] = fork();
+        if (pid[1] != 0)//parent process
         {
             FdUniquePtr pipeptr(pipefd2+1);
             close(pipefd2[0]);
@@ -2102,16 +2080,12 @@ RUNNER_MULTIPROCESS_TEST(security_manager_17_privacy_manager_delete_policy_for_s
             ssize_t written = TEMP_FAILURE_RETRY(write(pipefd2[1], &msg, sizeof(struct message)));
             RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-            //wait for child
-            RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
-
-            //wait for child
-            waitpid(-1, &result, 0);
+            waitPid(pid[1]);
 
             admin.adminCheck(check_start_bucket, false, generateAppLabel(update_app_id).c_str(),
                     std::to_string(static_cast<int>(msg.uid)).c_str(), update_privilege, CYNARA_ADMIN_DENY, nullptr);
         }
-        if(pid == 0)
+        if(pid[1] == 0)
         {
             FdUniquePtr pipeptr(pipefd2);
             close(pipefd2[1]);
@@ -2134,7 +2108,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_17_privacy_manager_delete_policy_for_s
             exit(0);
         }
     }
-    if(pid == 0)
+    if(pid[0] == 0)
     {
         FdUniquePtr pipeptr(pipefd);
         close(pipefd[1]);
@@ -2211,8 +2185,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_17_privacy_manager_fetch_whole_policy_
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid);
     }
     if(pid == 0)
     {
@@ -2442,8 +2415,7 @@ RUNNER_MULTIPROCESS_TEST(security_manager_21_security_manager_admin_deny_user_pr
         ssize_t written = TEMP_FAILURE_RETRY(write(pipefd[1], &msg, sizeof(struct message)));
         RUNNER_ASSERT_MSG((written == sizeof(struct message)),"write failed");
 
-        //wait for child
-        RUNNER_ASSERT_MSG(wait(&result) == pid, "wait failed");
+        waitPid(pid);
 
         check_app_permissions(app_id.c_str(), pkg_id.c_str(), childuidstr.c_str(),
                               real_privs_allow, real_privs_deny);