>From aca21045e7cef8ab7371ab5262a30021ddce8273 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Mon, 2 Apr 2012 17:22:16 -0400 Subject: [PATCH 1/3] sss_atomic_io: Do not fail reads with EPIPE if there is not enough data to read Also adds a unit test for sss_atomic_io() --- src/tests/util-tests.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/util.c | 3 +- 2 files changed, 208 insertions(+), 1 deletions(-) diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c index 3dc6ae66af7c82d840dd0382846cc3f0a4aa9290..9e39a4cbdf8e72f7ae61d349ea5cc1fd3748b989 100644 --- a/src/tests/util-tests.c +++ b/src/tests/util-tests.c @@ -25,11 +25,18 @@ #include #include #include +#include +#include +#include #include "util/util.h" #include "util/sss_utf8.h" #include "util/murmurhash3.h" #include "tests/common.h" +#define FILENAME_TEMPLATE "tests-atomicio-XXXXXX" +char *filename; +int atio_fd; + START_TEST(test_parse_args) { struct pa_testcase { @@ -445,6 +452,194 @@ START_TEST(test_murmurhash3_random) } END_TEST +void setup_atomicio(void) +{ + int ret; + mode_t old_umask; + + filename = strdup(FILENAME_TEMPLATE); + fail_unless(filename != NULL, "strdup failed"); + + atio_fd = -1; + old_umask = umask(077); + ret = mkstemp(filename); + umask(old_umask); + fail_unless(ret != -1, "mkstemp failed [%d][%s]", errno, strerror(errno)); + atio_fd = ret; +} + +void teardown_atomicio(void) +{ + int ret; + + if (atio_fd != -1) { + ret = close(atio_fd); + fail_unless(ret == 0, "close failed [%d][%s]", errno, strerror(errno)); + } + + fail_unless(filename != NULL, "unknown filename"); + ret = unlink(filename); + free(filename); + fail_unless(ret == 0, "unlink failed [%d][%s]", errno, strerror(errno)); +} + +START_TEST(test_atomicio_read_from_file) +{ + const ssize_t bufsize = 64; + char buf[64]; + int fd; + ssize_t numread; + errno_t ret; + + fd = open("/dev/zero", O_RDONLY); + fail_if(fd == -1, "Cannot open /dev/zero"); + + errno = 0; + numread = sss_atomic_read(fd, buf, bufsize); + ret = errno; + + fail_unless(ret == 0, "Error %d while reading\n", ret); + fail_unless(numread == bufsize, + "Read %d bytes expected %d\n", numread, bufsize); + close(fd); +} +END_TEST + +START_TEST(test_atomicio_read_from_small_file) +{ + char wbuf[] = "foobar"; + ssize_t wsize = strlen(wbuf)+1; + ssize_t numwritten; + char rbuf[64]; + ssize_t numread; + errno_t ret; + + fail_if(atio_fd < 0, "No fd to test?\n"); + + errno = 0; + numwritten = sss_atomic_write(atio_fd, wbuf, wsize); + ret = errno; + + fail_unless(ret == 0, "Error %d while writing\n", ret); + fail_unless(numwritten == wsize, + "Wrote %d bytes expected %d\n", numwritten, wsize); + + fsync(atio_fd); + lseek(atio_fd, 0, SEEK_SET); + + errno = 0; + numread = sss_atomic_read(atio_fd, rbuf, 64); + ret = errno; + + fail_unless(ret == 0, "Error %d while reading\n", ret); + fail_unless(numread == numwritten, + "Read %d bytes expected %d\n", numread, numwritten); +} +END_TEST + +START_TEST(test_atomicio_read_from_large_file) +{ + char wbuf[] = "123456781234567812345678"; + ssize_t wsize = strlen(wbuf)+1; + ssize_t numwritten; + char rbuf[8]; + ssize_t numread; + ssize_t total; + errno_t ret; + + fail_if(atio_fd < 0, "No fd to test?\n"); + + errno = 0; + numwritten = sss_atomic_write(atio_fd, wbuf, wsize); + ret = errno; + + fail_unless(ret == 0, "Error %d while writing\n", ret); + fail_unless(numwritten == wsize, + "Wrote %d bytes expected %d\n", numwritten, wsize); + + fsync(atio_fd); + lseek(atio_fd, 0, SEEK_SET); + + total = 0; + do { + errno = 0; + numread = sss_atomic_read(atio_fd, rbuf, 8); + ret = errno; + + fail_if(numread == -1, "Read error %d: %s\n", ret, strerror(ret)); + total += numread; + } while (numread != 0); + + fail_unless(ret == 0, "Error %d while reading\n", ret); + fail_unless(total == numwritten, + "Read %d bytes expected %d\n", numread, numwritten); +} +END_TEST + +START_TEST(test_atomicio_read_exact_sized_file) +{ + char wbuf[] = "12345678"; + ssize_t wsize = strlen(wbuf)+1; + ssize_t numwritten; + char rbuf[9]; + ssize_t numread; + errno_t ret; + + fail_if(atio_fd < 0, "No fd to test?\n"); + + errno = 0; + numwritten = sss_atomic_write(atio_fd, wbuf, wsize); + ret = errno; + + fail_unless(ret == 0, "Error %d while writing\n", ret); + fail_unless(numwritten == wsize, + "Wrote %d bytes expected %d\n", numwritten, wsize); + + fsync(atio_fd); + lseek(atio_fd, 0, SEEK_SET); + + errno = 0; + numread = sss_atomic_read(atio_fd, rbuf, 9); + ret = errno; + + fail_unless(ret == 0, "Error %d while reading\n", ret); + fail_unless(numread == numwritten, + "Read %d bytes expected %d\n", numread, numwritten); + + fail_unless(rbuf[8] == '\0', "String not NULL terminated?"); + fail_unless(strcmp(wbuf, rbuf) == 0, "Read something else than wrote?"); + + /* We've reached end-of-file, next read must return 0 */ + errno = 0; + numread = sss_atomic_read(atio_fd, rbuf, 9); + ret = errno; + + fail_unless(ret == 0, "Error %d while reading\n", ret); + fail_unless(numread == 0, "More data to read?"); +} +END_TEST + +START_TEST(test_atomicio_read_from_empty_file) +{ + char buf[64]; + int fd; + ssize_t numread; + errno_t ret; + + fd = open("/dev/null", O_RDONLY); + fail_if(fd == -1, "Cannot open /dev/null"); + + errno = 0; + numread = sss_atomic_read(fd, buf, 64); + ret = errno; + + fail_unless(ret == 0, "Error %d while reading\n", ret); + fail_unless(numread == 0, + "Read %d bytes expected 0\n", numread); + close(fd); +} +END_TEST + Suite *util_suite(void) { Suite *s = suite_create("util"); @@ -471,9 +666,20 @@ Suite *util_suite(void) tcase_add_test (tc_mh3, test_murmurhash3_random); tcase_set_timeout(tc_mh3, 60); + TCase *tc_atomicio = tcase_create("atomicio"); + tcase_add_checked_fixture (tc_atomicio, + setup_atomicio, + teardown_atomicio); + tcase_add_test(tc_atomicio, test_atomicio_read_from_file); + tcase_add_test(tc_atomicio, test_atomicio_read_from_small_file); + tcase_add_test(tc_atomicio, test_atomicio_read_from_large_file); + tcase_add_test(tc_atomicio, test_atomicio_read_exact_sized_file); + tcase_add_test(tc_atomicio, test_atomicio_read_from_empty_file); + suite_add_tcase (s, tc_util); suite_add_tcase (s, tc_utf8); suite_add_tcase (s, tc_mh3); + suite_add_tcase (s, tc_atomicio); return s; } diff --git a/src/util/util.c b/src/util/util.c index 8e20ab714ccdd3498034b934530e4cde5782b554..3a6c5d270bf02d265f7dc0c772f51766105ecb94 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -639,7 +639,8 @@ ssize_t sss_atomic_io(int fd, void *buf, size_t n, bool do_read) } return -1; case 0: - errno = EPIPE; + /* read returns 0 on end-of-file */ + errno = do_read ? 0 : EPIPE; return pos; default: pos += (size_t) res; -- 1.7.7.6