[SSSD] [PATCH] Fix segmentation fault in test_io.

Lukas Slebodnik lslebodn at redhat.com
Thu Apr 11 11:47:03 UTC 2013


On (11/04/13 11:18), Jakub Hrozek wrote:
>On Thu, Apr 11, 2013 at 11:04:06AM +0200, Lukas Slebodnik wrote:
>> I wanted to revie som patches, but test failed with unrelated issue to revieved
>> patch.
>>
>
>The test_io.c test should also call tests_set_cwd().
>
>Is this only an issue with test_io.c ? Did sysdb_tests run OK? I would
>expect them to have the same problem, can you check?

I fixed also tests_set_cwd(), there was ignored return value if TEST_DIR is empty
string. It should not happen with default TEST_DIR value.

Attached patch just fix segmentation fault and improve some lines to prevent
segfault.

Abhishek can rewrite test_io.c to using tests_set_cwd(),
(use relative paths)
Also there something what is not very good solution (transformation to relativepath)

    relativepath = strchr(get_filepath(path), 't');
                                             ^^^^^
What if I decide to call ./configure --with-test-dir="temp_test"?

LS
-------------- next part --------------
>From 8f247b6a38b9ebc0db92f1209b8e54d7f4f65e55 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Thu, 11 Apr 2013 10:00:14 +0200
Subject: [PATCH] Fix segmentation fault in test_io.

If configure isn't being run with argument --with-test-dir, then variable
TEST_DIR will be defined, but its value will be empty (""). In this case
opendir will fail with uncatched error "Directory does not exist, or name is
an empty string". Finally function call dirfd will segfault because its
argument is NULL.

I changed default value of TEST_DIR (if --with-test-dir was not used).
Assert to auxiliary function was added and error messages became more verbose.

Function tests_set_cwd does not ignore return value of chdir, because
TEST_DIR should not be the empty string.
---
 src/conf_macros.m4         |  7 ++++---
 src/tests/cmocka/test_io.c | 13 ++++++++++---
 src/tests/common.c         |  8 ++------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index 4ea4e93679563b7c3fdc9af751b1cafd728f859b..26eb4acccc4895e961ecc50d490a3cdd5716d085 100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -329,11 +329,12 @@ AC_DEFUN([WITH_TEST_DIR],
                 [AC_HELP_STRING([--with-test-dir=PATH],
                                 [Directory used for make check temporary files [$builddir]]
                                )
-                ]
+                ],
+                [TEST_DIR=$withval],
+                [TEST_DIR="."]
                )
-    TEST_DIR=$with_test_dir
     AC_SUBST(TEST_DIR)
-    AC_DEFINE_UNQUOTED(TEST_DIR, "$with_test_dir", [Directory used for 'make check' temporary files])
+    AC_DEFINE_UNQUOTED(TEST_DIR, "$TEST_DIR", [Directory used for 'make check' temporary files])
   ])
 
 AC_DEFUN([WITH_NSCD],
diff --git a/src/tests/cmocka/test_io.c b/src/tests/cmocka/test_io.c
index 4d21b2e25d3f35cc155b6410bcb4ce8963a59c15..5418ac7935e1145ea66e8396d164ea6e01635959 100644
--- a/src/tests/cmocka/test_io.c
+++ b/src/tests/cmocka/test_io.c
@@ -49,8 +49,11 @@ static char *get_filepath(char path[])
     ret = mkstemp(path);
 
     if (ret == -1) {
-        fprintf(stderr, "mkstemp failed\n");
+        int err = errno;
+        fprintf(stderr, "mkstemp failed with path:'%s' [%s]\n",
+                path, strerror(err));
     }
+    assert_false(ret == -1);
 
     return path;
 }
@@ -58,9 +61,13 @@ static char *get_filepath(char path[])
 void setup_dirp(void **state)
 {
     DIR *dirp = opendir(TEST_DIR);
-    if (dirp != NULL){
-        *state = (void *)dirp;
+    if (dirp == NULL) {
+        int err = errno;
+        fprintf(stderr, "Could not open directory:'%s' [%s]\n",
+                TEST_DIR, strerror(err));
     }
+    assert_non_null(dirp);
+    *state = (void *)dirp;
 }
 
 void teardown_dirp(void **state)
diff --git a/src/tests/common.c b/src/tests/common.c
index 54c93e281d166e7b55a27a96f5dbc7ce67c57e62..16c051656f86f7b0914893a9349366893429e55f 100644
--- a/src/tests/common.c
+++ b/src/tests/common.c
@@ -33,12 +33,8 @@ tests_set_cwd(void)
 
     ret = chdir(TEST_DIR);
     if (ret == -1) {
-        if (strlen(TEST_DIR)) {
-            fprintf(stderr,
-                    "Could not chdir to [%s].\n"
-                    "Attempting to continue with current dir\n",
-                    TEST_DIR);
-        }
+        fprintf(stderr, "Could not chdir to [%s].\n"
+                "Attempting to continue with current dir\n", TEST_DIR);
     }
 }
 
-- 
1.8.1.4



More information about the sssd-devel mailing list