[SSSD] [PATCH] Convert read and write operations to sss_atomic_{read|write}

Jakub Hrozek jhrozek at redhat.com
Thu Apr 12 13:13:27 UTC 2012


On Thu, Apr 12, 2012 at 08:16:32AM -0400, Stephen Gallagher wrote:
> On Thu, 2012-04-12 at 13:45 +0200, Jakub Hrozek wrote:
> > On Tue, Apr 10, 2012 at 08:21:32AM -0400, Simo Sorce wrote:
> > > On Tue, 2012-04-10 at 05:36 -0400, Jakub Hrozek wrote:
> > > > Andreas, can you check the src/util/util.c hunk in the first patch? I
> > > > think the atomic io functions were taken from libssh that you maintain..
> > > > 
> > > > Honza, can you check out if I didn't miss anything in sss_ssh_knownproxy
> > > > and if it can in fact use the atomic read?
> > > > 
> > > > [PATCH 1/3] sss_atomic_io: Do not fail reads with EPIPE if there is not
> > > > enough data to read
> > > > The hunk in src/util/util.c would apply when the read buffer is bigger
> > > > than the contents of the file we read from. The rest is just unit tests.
> > > > 
> > > > [PATCH 2/3] Move atomic io function to a separate module
> > > > We'll be using it on various places of the SSSD. The function is in its
> > > > own file to allow using just the one piece without having to drag in the
> > > > whole util.c module.
> > > > There is no functional change in this patch
> > > > 
> > > > [PATCH 3/3] Convert read and write operations to sss_atomic_read
> > > > https://fedorahosted.org/sssd/ticket/1209
> > > > 
> > > > There are two exceptions that were not converted - one is the read in
> > > > sss_ssh_knownproxy because sss_ssh_knownproxy uses its own poll logic
> > > > that seemed to interfere with what atomic_read_does. The other are read
> > > > and write loops in the sss_client. Those use a logic as to how many bytes
> > > > are left to read and also handle polling themselves.
> > > 
> > > Can we call these functions "blocking_io" instead of atomic_io ?
> > > We need to make very clear these functions do block potentially for a
> > > long time and cannot be used in the async code paths.
> > > 
> > 
> > I agree, but "blocking_io" seems too strong to me, what about a _s
> > suffix (like in openldap) and a comment in the header? See attached
> > patches.
> 
> I think you may have sent the old patches. These don't include either of
> the changes you proposed (at a quick glance).

Oops, I did. The new patches are attached now.
-------------- next part --------------
>From 365b85b3f54e2ad12df5290cdf0cfe8697e43104 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
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 <popt.h>
 #include <talloc.h>
 #include <check.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #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

-------------- next part --------------
>From 2022268588ae35351d4fe7977b281d07f2179dd4 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 9 Apr 2012 23:30:58 +0200
Subject: [PATCH 2/3] Move atomic io function to a separate module

We'll be using it on various places of the SSSD. The function is in its
own file to allow using just the one piece without having to drag in the
whole util.c module.
---
 Makefile.am            |    2 +
 src/tests/util-tests.c |   18 +++++++-------
 src/util/atomic_io.c   |   60 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/util/atomic_io.h   |   40 ++++++++++++++++++++++++++++++++
 src/util/util.c        |   38 ------------------------------
 src/util/util.h        |    7 +----
 6 files changed, 113 insertions(+), 52 deletions(-)
 create mode 100644 src/util/atomic_io.c
 create mode 100644 src/util/atomic_io.h

diff --git a/Makefile.am b/Makefile.am
index c62ea8c1c5d21cc7234a48d33d96bf54b540780c..919fb9b2516088ee5ad9daabd37a9d7d282aeb21 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -332,6 +332,7 @@ dist_noinst_HEADERS = \
     src/util/user_info_msg.h \
     src/util/murmurhash3.h \
     src/util/mmap_cache.h \
+    src/util/atomic_io.h \
     src/monitor/monitor.h \
     src/monitor/monitor_interfaces.h \
     src/responder/common/responder.h \
@@ -449,6 +450,7 @@ libsss_util_la_SOURCES = \
     src/util/sss_utf8.c \
     src/util/sss_tc_utf8.c \
     src/util/murmurhash3.c \
+    src/util/atomic_io.c \
     src/util/sss_selinux.c
 libsss_util_la_LIBADD = \
     $(SSSD_LIBS) \
diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
index 9e39a4cbdf8e72f7ae61d349ea5cc1fd3748b989..59c80ca6f2c5432bfc5f85ecec745fd28b38637b 100644
--- a/src/tests/util-tests.c
+++ b/src/tests/util-tests.c
@@ -495,7 +495,7 @@ START_TEST(test_atomicio_read_from_file)
     fail_if(fd == -1, "Cannot open /dev/zero");
 
     errno = 0;
-    numread = sss_atomic_read(fd, buf, bufsize);
+    numread = sss_atomic_read_s(fd, buf, bufsize);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while reading\n", ret);
@@ -517,7 +517,7 @@ START_TEST(test_atomicio_read_from_small_file)
     fail_if(atio_fd < 0, "No fd to test?\n");
 
     errno = 0;
-    numwritten = sss_atomic_write(atio_fd, wbuf, wsize);
+    numwritten = sss_atomic_write_s(atio_fd, wbuf, wsize);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while writing\n", ret);
@@ -528,7 +528,7 @@ START_TEST(test_atomicio_read_from_small_file)
     lseek(atio_fd, 0, SEEK_SET);
 
     errno = 0;
-    numread = sss_atomic_read(atio_fd, rbuf, 64);
+    numread = sss_atomic_read_s(atio_fd, rbuf, 64);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while reading\n", ret);
@@ -550,7 +550,7 @@ START_TEST(test_atomicio_read_from_large_file)
     fail_if(atio_fd < 0, "No fd to test?\n");
 
     errno = 0;
-    numwritten = sss_atomic_write(atio_fd, wbuf, wsize);
+    numwritten = sss_atomic_write_s(atio_fd, wbuf, wsize);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while writing\n", ret);
@@ -563,7 +563,7 @@ START_TEST(test_atomicio_read_from_large_file)
     total = 0;
     do {
         errno = 0;
-        numread = sss_atomic_read(atio_fd, rbuf, 8);
+        numread = sss_atomic_read_s(atio_fd, rbuf, 8);
         ret = errno;
 
         fail_if(numread == -1, "Read error %d: %s\n", ret, strerror(ret));
@@ -588,7 +588,7 @@ START_TEST(test_atomicio_read_exact_sized_file)
     fail_if(atio_fd < 0, "No fd to test?\n");
 
     errno = 0;
-    numwritten = sss_atomic_write(atio_fd, wbuf, wsize);
+    numwritten = sss_atomic_write_s(atio_fd, wbuf, wsize);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while writing\n", ret);
@@ -599,7 +599,7 @@ START_TEST(test_atomicio_read_exact_sized_file)
     lseek(atio_fd, 0, SEEK_SET);
 
     errno = 0;
-    numread = sss_atomic_read(atio_fd, rbuf, 9);
+    numread = sss_atomic_read_s(atio_fd, rbuf, 9);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while reading\n", ret);
@@ -611,7 +611,7 @@ START_TEST(test_atomicio_read_exact_sized_file)
 
     /* We've reached end-of-file, next read must return 0 */
     errno = 0;
-    numread = sss_atomic_read(atio_fd, rbuf, 9);
+    numread = sss_atomic_read_s(atio_fd, rbuf, 9);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while reading\n", ret);
@@ -630,7 +630,7 @@ START_TEST(test_atomicio_read_from_empty_file)
     fail_if(fd == -1, "Cannot open /dev/null");
 
     errno = 0;
-    numread = sss_atomic_read(fd, buf, 64);
+    numread = sss_atomic_read_s(fd, buf, 64);
     ret = errno;
 
     fail_unless(ret == 0, "Error %d while reading\n", ret);
diff --git a/src/util/atomic_io.c b/src/util/atomic_io.c
new file mode 100644
index 0000000000000000000000000000000000000000..1543af9a0286c57028f3cad63fdae6e58a5ba539
--- /dev/null
+++ b/src/util/atomic_io.c
@@ -0,0 +1,60 @@
+/*
+    Authors:
+        Jan Cholasta <jcholast at redhat.com>
+
+    Copyright (C) 2012 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "util/atomic_io.h"
+
+/* based on code from libssh <http://www.libssh.org> */
+ssize_t sss_atomic_io_s(int fd, void *buf, size_t n, bool do_read)
+{
+    char *b = buf;
+    size_t pos = 0;
+    ssize_t res;
+    struct pollfd pfd;
+
+    pfd.fd = fd;
+    pfd.events = do_read ? POLLIN : POLLOUT;
+
+    while (n > pos) {
+        if (do_read) {
+            res = read(fd, b + pos, n - pos);
+        } else {
+            res = write(fd, b + pos, n - pos);
+        }
+        switch (res) {
+        case -1:
+            if (errno == EINTR) {
+                continue;
+            }
+            if (errno == EAGAIN || errno == EWOULDBLOCK) {
+                (void) poll(&pfd, 1, -1);
+                continue;
+            }
+            return -1;
+        case 0:
+            /* read returns 0 on end-of-file */
+            errno = do_read ? 0 : EPIPE;
+            return pos;
+        default:
+            pos += (size_t) res;
+        }
+    }
+
+    return pos;
+}
diff --git a/src/util/atomic_io.h b/src/util/atomic_io.h
new file mode 100644
index 0000000000000000000000000000000000000000..ffae31d6c252674c7b24e752fe9b2c8415e72e15
--- /dev/null
+++ b/src/util/atomic_io.h
@@ -0,0 +1,40 @@
+/*
+    Authors:
+        Jan Cholasta <jcholast at redhat.com>
+
+    Copyright (C) 2012 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef __SSSD_ATOMIC_IO_H__
+#define __SSSD_ATOMIC_IO_H__
+
+#include <unistd.h>
+#include <stdbool.h>
+#include <poll.h>
+#include <errno.h>
+
+/* Performs a read or write operation in an manner that is seemingly atomic
+ * to the caller.
+ *
+ * Please note that the function does not perform any asynchronous operation
+ * so the operation might potentially block
+ */
+ssize_t sss_atomic_io_s(int fd, void *buf, size_t n, bool do_read);
+
+#define sss_atomic_read_s(fd, buf, n)  sss_atomic_io_s(fd, buf, n, true)
+#define sss_atomic_write_s(fd, buf, n) sss_atomic_io_s(fd, buf, n, false)
+
+#endif /* __SSSD_ATOMIC_IO_H__ */
diff --git a/src/util/util.c b/src/util/util.c
index 3a6c5d270bf02d265f7dc0c772f51766105ecb94..f1aaebc28addf3e468136ba35b375883046bf8a1 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -611,41 +611,3 @@ void to_sized_string(struct sized_string *out, const char *in)
     }
 }
 
-/* based on code from libssh <http://www.libssh.org> */
-ssize_t sss_atomic_io(int fd, void *buf, size_t n, bool do_read)
-{
-    char *b = buf;
-    size_t pos = 0;
-    ssize_t res;
-    struct pollfd pfd;
-
-    pfd.fd = fd;
-    pfd.events = do_read ? POLLIN : POLLOUT;
-
-    while (n > pos) {
-        if (do_read) {
-            res = read(fd, b + pos, n - pos);
-        } else {
-            res = write(fd, b + pos, n - pos);
-        }
-        switch (res) {
-        case -1:
-            if (errno == EINTR) {
-                continue;
-            }
-            if (errno == EAGAIN || errno == EWOULDBLOCK) {
-                (void) poll(&pfd, 1, -1);
-                continue;
-            }
-            return -1;
-        case 0:
-            /* read returns 0 on end-of-file */
-            errno = do_read ? 0 : EPIPE;
-            return pos;
-        default:
-            pos += (size_t) res;
-        }
-    }
-
-    return pos;
-}
diff --git a/src/util/util.h b/src/util/util.h
index da6db1cffe36035f27c447b9971b4a8ba7b7f740..181e75166764057be093463c86c05b911b0488eb 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -42,6 +42,8 @@
 #include <ldb.h>
 #include <dhash.h>
 
+#include "util/atomic_io.h"
+
 #ifndef HAVE_ERRNO_T
 #define HAVE_ERRNO_T
 typedef int errno_t;
@@ -510,11 +512,6 @@ errno_t sss_filter_sanitize(TALLOC_CTX *mem_ctx,
 char *
 sss_escape_ip_address(TALLOC_CTX *mem_ctx, int family, const char *addr);
 
-ssize_t sss_atomic_io(int fd, void *buf, size_t n, bool do_read);
-
-#define sss_atomic_read(fd, buf, n)  sss_atomic_io(fd, buf, n, true)
-#define sss_atomic_write(fd, buf, n) sss_atomic_io(fd, buf, n, false)
-
 /* from sss_tc_utf8.c */
 char *
 sss_tc_utf8_str_tolower(TALLOC_CTX *mem_ctx, const char *s);
-- 
1.7.7.6

-------------- next part --------------
>From 4bd4d57e34953a593810e38e53bbb23b888a0282 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 2 Apr 2012 17:26:05 -0400
Subject: [PATCH 3/3] Convert read and write operations to sss_atomic_read

https://fedorahosted.org/sssd/ticket/1209
---
 Makefile.am                                  |    7 ++-
 src/krb5_plugin/sssd_krb5_locator_plugin.c   |   22 +++----
 src/monitor/monitor.c                        |   41 ++++++------
 src/monitor/monitor_netlink.c                |   20 +++---
 src/providers/krb5/krb5_child.c              |   51 ++++++--------
 src/providers/krb5/krb5_common.c             |   26 +++----
 src/providers/ldap/ldap_child.c              |   53 ++++++---------
 src/responder/nss/nsssrv_mmap_cache.c        |   20 +++---
 src/responder/ssh/sshsrv_cmd.c               |    2 +-
 src/sss_client/pam_sss.c                     |   50 +++++++-------
 src/sss_client/ssh/sss_ssh_knownhostsproxy.c |    4 +-
 src/tools/files.c                            |   49 +++++--------
 src/util/backup_file.c                       |   40 +++++------
 src/util/child_common.c                      |   53 ++++++--------
 src/util/find_uid.c                          |   16 ++---
 src/util/server.c                            |   96 +++++++++++---------------
 16 files changed, 239 insertions(+), 311 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 919fb9b2516088ee5ad9daabd37a9d7d282aeb21..8293c08ee1515ec7c7b682185bf279ea77dccb33 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -777,6 +777,7 @@ endif
 files_tests_SOURCES = \
     src/tests/files-tests.c \
     src/util/check_and_open.c \
+    src/util/atomic_io.c \
     src/tools/selinux.c \
     src/tools/files.c
 files_tests_CFLAGS = \
@@ -835,6 +836,7 @@ fail_over_tests_LDADD = \
 find_uid_tests_SOURCES = \
     src/tests/find_uid-tests.c \
     src/util/find_uid.c \
+    src/util/atomic_io.c \
     src/util/strtonum.c
 find_uid_tests_CFLAGS = \
     $(AM_CFLAGS) \
@@ -1225,6 +1227,7 @@ krb5_child_SOURCES = \
     src/providers/dp_pam_data_util.c \
     src/util/user_info_msg.c \
     src/util/sss_krb5.c \
+    src/util/atomic_io.c \
     src/util/util.c \
     src/util/signal.c
 krb5_child_CFLAGS = \
@@ -1243,6 +1246,7 @@ krb5_child_LDADD = \
 ldap_child_SOURCES = \
     src/providers/ldap/ldap_child.c \
     src/util/sss_krb5.c \
+    src/util/atomic_io.c \
     src/util/util.c \
     src/util/signal.c
 ldap_child_CFLAGS = \
@@ -1284,7 +1288,8 @@ memberof_la_LDFLAGS = \
 
 if BUILD_KRB5_LOCATOR_PLUGIN
 sssd_krb5_locator_plugin_la_SOURCES = \
-    src/krb5_plugin/sssd_krb5_locator_plugin.c
+    src/krb5_plugin/sssd_krb5_locator_plugin.c \
+    src/util/atomic_io.c
 sssd_krb5_locator_plugin_la_CFLAGS = \
     $(AM_CFLAGS) \
     $(KRB5_CFLAGS)
diff --git a/src/krb5_plugin/sssd_krb5_locator_plugin.c b/src/krb5_plugin/sssd_krb5_locator_plugin.c
index 3eced04d67bd8f90b88a0a2ca8a870eac2e1ea53..b8d4e31b654c3961b56b0505209bf45675a63241 100644
--- a/src/krb5_plugin/sssd_krb5_locator_plugin.c
+++ b/src/krb5_plugin/sssd_krb5_locator_plugin.c
@@ -86,7 +86,6 @@ static int get_krb5info(const char *realm, struct sssd_ctx *ctx,
     char *krb5info_name = NULL;
     size_t len;
     uint8_t buf[BUFSIZE + 1];
-    uint8_t *p;
     int fd = -1;
     const char *name_tmpl = NULL;
     char *port_str;
@@ -129,23 +128,19 @@ static int get_krb5info(const char *realm, struct sssd_ctx *ctx,
         goto done;
     }
 
-    len = BUFSIZE;
-    p = buf;
     memset(buf, 0, BUFSIZE+1);
-    while (len != 0 && (ret = read(fd, p, len)) != 0) {
-        if (ret == -1) {
-            if (errno == EINTR || errno == EAGAIN) continue;
-            PLUGIN_DEBUG(("read failed [%d][%s].\n", errno, strerror(errno)));
-            close(fd);
-            goto done;
-        }
 
-        len -= ret;
-        p += ret;
+    errno = 0;
+    len = sss_atomic_read_s(fd, buf, BUFSIZE);
+    if (len == -1) {
+        ret = errno;
+        PLUGIN_DEBUG(("read failed [%d][%s].\n", ret, strerror(ret)));
+        close(fd);
+        goto done;
     }
     close(fd);
 
-    if (len == 0) {
+    if (len == BUFSIZE) {
         PLUGIN_DEBUG(("Content of krb5info file [%s] is [%d] or larger.\n",
                       krb5info_name, BUFSIZE));
     }
@@ -212,6 +207,7 @@ static int get_krb5info(const char *realm, struct sssd_ctx *ctx,
             goto done;
     }
 
+    ret = 0;
 done:
     free(krb5info_name);
     return ret;
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index a93b23460242a7e2144afa375157ac77fddd2f6c..39b831a8fad18d8a8f2a411f9b04d4a24649c3ba 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -1478,11 +1478,12 @@ static void process_config_file(struct tevent_context *ev,
     struct inotify_event *in_event;
     char *buf;
     char *name;
-    ssize_t len, total_len;
+    ssize_t len;
     ssize_t event_size;
     struct config_file_ctx *file_ctx;
     struct config_file_callback *cb;
     struct rewatch_ctx *rw_ctx;
+    errno_t ret;
 
     event_size = sizeof(struct inotify_event);
     file_ctx = talloc_get_type(ptr, struct config_file_ctx);
@@ -1497,16 +1498,14 @@ static void process_config_file(struct tevent_context *ev,
         goto done;
     }
 
-    total_len = 0;
-    while (total_len < event_size) {
-        len = read(file_ctx->mt_ctx->inotify_fd, buf+total_len,
-                   event_size-total_len);
-        if (len == -1) {
-            if (errno == EINTR || errno == EAGAIN) continue;
-            DEBUG(0, ("Critical error reading inotify file descriptor.\n"));
-            goto done;
-        }
-        total_len += len;
+    errno = 0;
+    len = sss_atomic_read_s(file_ctx->mt_ctx->inotify_fd, buf, event_size);
+    if (len == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Critical error reading inotify file descriptor [%d]: %s\n",
+               ret, strerror(ret)));
+        goto done;
     }
 
     in_event = (struct inotify_event *)buf;
@@ -1515,19 +1514,19 @@ static void process_config_file(struct tevent_context *ev,
         /* Read in the name, even though we don't use it,
          * so that read ptr is in the right place
          */
-        name = talloc_size(tmp_ctx, len);
+        name = talloc_size(tmp_ctx, in_event->len);
         if (!name) {
             goto done;
         }
-        total_len = 0;
-        while (total_len < in_event->len) {
-            len = read(file_ctx->mt_ctx->inotify_fd, &name, in_event->len);
-            if (len == -1) {
-                if (errno == EINTR || errno == EAGAIN) continue;
-                DEBUG(0, ("Critical error reading inotify file descriptor.\n"));
-                goto done;
-            }
-            total_len += len;
+
+        errno = 0;
+        len = sss_atomic_read_s(file_ctx->mt_ctx->inotify_fd, name, in_event->len);
+        if (len == -1) {
+            ret = errno;
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                ("Critical error reading inotify file descriptor [%d]: %s\n",
+                ret, strerror(ret)));
+            goto done;
         }
     }
 
diff --git a/src/monitor/monitor_netlink.c b/src/monitor/monitor_netlink.c
index 2fe380ac9a9ee0096e753550bdad2d200c6c73af..3842c4f1612367ab2d33a1000d81fd29506fbf4d 100644
--- a/src/monitor/monitor_netlink.c
+++ b/src/monitor/monitor_netlink.c
@@ -153,19 +153,17 @@ static bool has_ethernet_encapsulation(const char *sysfs_path)
     }
 
     memset(buf, 0, BUFSIZE);
-    while ((ret = read(fd, buf, BUFSIZE)) != 0) {
-        if (ret == -1) {
-            ret = errno;
-            if (ret == EINTR || ret == EAGAIN) {
-                continue;
-            }
-            DEBUG(SSSDBG_OP_FAILURE,
-                  ("read failed [%d][%s].\n", ret, strerror(ret)));
-            close(fd);
-            return false;
-        }
+    errno = 0;
+    ret = sss_atomic_read_s(fd, buf, BUFSIZE);
+    if (ret == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("read failed [%d][%s].\n", ret, strerror(ret)));
+        close(fd);
+        return false;
     }
     close(fd);
+    buf[BUFSIZE-1] = '\0';
 
     return strncmp(buf, "1\n", BUFSIZE) == 0;
 }
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 209643a096c8eb9f5fe572cbc7891c4623be29fe..cc29c00f6a2d128f1bb3d5878eda334adad2af85 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -461,18 +461,20 @@ static errno_t sendresponse(int fd, krb5_error_code kerr, int pam_status,
         return ENOMEM;
     }
 
-    written = 0;
-    while (written < resp->size) {
-        ret = write(fd, resp->buf + written, resp->size - written);
-        if (ret == -1) {
-            if (errno == EAGAIN || errno == EINTR) {
-                continue;
-            }
-            ret = errno;
-            DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret)));
-            return ret;
-        }
-        written += ret;
+    errno = 0;
+    written = sss_atomic_write_s(fd, resp->buf, resp->size);
+    if (written == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("write failed [%d][%s].\n", ret, strerror(ret)));
+        return ret;
+    }
+
+    if (written != resp->size) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Write error, wrote [%d] bytes, expected [%d]\n",
+               written, resp->size));
+        return EOK;
     }
 
     return EOK;
@@ -1631,25 +1633,14 @@ int main(int argc, const char *argv[])
         goto fail;
     }
 
-    while ((ret = read(STDIN_FILENO, buf + len, IN_BUF_SIZE - len)) != 0) {
-        if (ret == -1) {
-            if (errno == EINTR || errno == EAGAIN) {
-                continue;
-            }
-            DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
-            goto fail;
-        } else if (ret > 0) {
-            len += ret;
-            if (len > IN_BUF_SIZE) {
-                DEBUG(1, ("read too much, this should never happen.\n"));
-                goto fail;
-            }
-            continue;
-        } else {
-            DEBUG(1, ("unexpected return code of read [%d].\n", ret));
-            goto fail;
-        }
+    errno = 0;
+    len = sss_atomic_read_s(STDIN_FILENO, buf, IN_BUF_SIZE);
+    if (len == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("read failed [%d][%s].\n", ret, strerror(ret)));
+        goto fail;
     }
+
     close(STDIN_FILENO);
 
     kr = talloc_zero(pd, struct krb5_req);
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index d33900f18beb607a81aab5a74243ebb504202710..022745d6153c524cf30dbf787c3343d874cde90a 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -321,25 +321,19 @@ errno_t write_krb5info_file(const char *realm, const char *server,
         goto done;
     }
 
-    written = 0;
-    while (written < server_len) {
-        ret = write(fd, server+written, server_len-written);
-        if (ret == -1) {
-            if (errno == EINTR || errno == EAGAIN) {
-                continue;
-            }
-            ret = errno;
-            DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret)));
-            goto done;
-        }
-        else {
-            written += ret;
-        }
+    errno = 0;
+    written = sss_atomic_write_s(fd, discard_const(server), server_len);
+    if (written == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("write failed [%d][%s].\n", ret, strerror(ret)));
+        goto done;
     }
 
     if (written != server_len) {
-        DEBUG(1, ("Write error, wrote [%d] bytes, expected [%d]\n",
-                   written, server_len));
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Write error, wrote [%d] bytes, expected [%d]\n",
+               written, server_len));
         ret = EIO;
         goto done;
     }
diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index e66406c0ee95501a9493d7df7783f97900b05ab3..5356f8834ef05bc74a64a869e72e65b89c8eac0b 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -446,25 +446,14 @@ int main(int argc, const char *argv[])
         goto fail;
     }
 
-    while ((ret = read(STDIN_FILENO, buf + len, IN_BUF_SIZE - len)) != 0) {
-        if (ret == -1) {
-            if (errno == EINTR || errno == EAGAIN) {
-                continue;
-            }
-            DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
-            goto fail;
-        } else if (ret > 0) {
-            len += ret;
-            if (len > IN_BUF_SIZE) {
-                DEBUG(1, ("read too much, this should never happen.\n"));
-                goto fail;
-            }
-            continue;
-        } else {
-            DEBUG(1, ("unexpected return code of read [%d].\n", ret));
-            goto fail;
-        }
+    errno = 0;
+    len = sss_atomic_read_s(STDIN_FILENO, buf, IN_BUF_SIZE);
+    if (len == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("read failed [%d][%s].\n", ret, strerror(ret)));
+        goto fail;
     }
+
     close(STDIN_FILENO);
 
     ret = unpack_buffer(buf, len, ibuf);
@@ -484,22 +473,22 @@ int main(int argc, const char *argv[])
 
     ret = prepare_response(main_ctx, ccname, expire_time, kerr, &resp);
     if (ret != EOK) {
-        DEBUG(1, ("prepare_response failed. [%d][%s].\n", ret, strerror(ret)));
-        return ENOMEM;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("prepare_response failed. [%d][%s].\n", ret, strerror(ret)));
+        goto fail;
     }
 
-    written = 0;
-    while (written < resp->size) {
-        ret = write(STDOUT_FILENO, resp->buf + written, resp->size - written);
-        if (ret == -1) {
-            if (errno == EAGAIN || errno == EINTR) {
-                continue;
-            }
-            ret = errno;
-            DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret)));
-            return ret;
-        }
-        written += ret;
+    errno = 0;
+    written = sss_atomic_write_s(STDOUT_FILENO, resp->buf, resp->size);
+    if (written == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("write failed [%d][%s].\n", ret, strerror(ret)));
+        goto fail;
+    }
+
+    if (written != resp->size) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Expected to write %d bytes, wrote %d\n",
+              resp->size, written));
+        goto fail;
     }
 
     close(STDOUT_FILENO);
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index 7a7a498b924b75da6f17070b231e5377a688e4ed..e60d06198bb828149b6bf2c5002a303a915d2f2a 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -512,16 +512,16 @@ static errno_t sss_mc_set_recycled(int fd)
         /* What do we do now ? */
         return errno;
     }
-    pos = 0;
-    while (pos < sizeof(h.status)) {
-        ret = write(fd, ((uint8_t *)&w) + pos, sizeof(h.status) - pos);
-        if (ret == -1) {
-            if (errno != EINTR) {
-                return errno;
-            }
-            continue;
-        }
-        pos += ret;
+
+    errno = 0;
+    ret = sss_atomic_write_s(fd, (uint8_t *)&w, sizeof(h.status));
+    if (ret == -1) {
+        return errno;
+    }
+
+    if (ret != sizeof(h.status)) {
+        /* Write error */
+        return EIO;
     }
 
     return EOK;
diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c
index 4882bacf93565c174723a3705c6a4b1e9a3b93d1..149137070892a1e00afe71347c92049f831ee541 100644
--- a/src/responder/ssh/sshsrv_cmd.c
+++ b/src/responder/ssh/sshsrv_cmd.c
@@ -516,7 +516,7 @@ ssh_host_pubkeys_update_known_hosts(struct ssh_cmd_ctx *cmd_ctx)
                     goto done;
                 }
 
-                wret = sss_atomic_write(fd, line, strlen(line));
+                wret = sss_atomic_write_s(fd, line, strlen(line));
                 if (wret == -1) {
                     ret = errno;
                     goto done;
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 4fe4613aa70f6a46fcb95c6b52553de2ea70d0e9..7084840abf9cc262007337d0b46b79d1f3cc4303 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -47,6 +47,7 @@
 #include "sss_pam_macros.h"
 
 #include "sss_cli.h"
+#include "util/atomic_io.h"
 
 #include <libintl.h>
 #define _(STRING) dgettext (PACKAGE, STRING)
@@ -499,17 +500,12 @@ static errno_t display_pw_reset_message(pam_handle_t *pamh,
         goto done;
     }
 
-
-    total_len = 0;
-    while (total_len < stat_buf.st_size) {
-        ret = read(fd, msg_buf + total_len, stat_buf.st_size - total_len);
-        if (ret == -1) {
-            if (errno == EINTR || errno == EAGAIN) continue;
-            ret = errno;
-            D(("read failed [%d][%s].", ret, strerror(ret)));
-            goto done;
-        }
-        total_len += ret;
+    errno = 0;
+    total_len = sss_atomic_read_s(fd, msg_buf, stat_buf.st_size);
+    if (ret == -1) {
+        ret = errno;
+        D(("read failed [%d][%s].", ret, strerror(ret)));
+        goto done;
     }
 
     ret = close(fd);
@@ -1088,7 +1084,8 @@ static int send_and_receive(pam_handle_t *pamh, struct pam_items *pi,
 #ifdef HAVE_SELINUX
     char *path = NULL;
     char *tmp_path = NULL;
-    int pos, len;
+    ssize_t written;
+    int len;
     int fd;
     mode_t oldmask;
 #endif /* HAVE_SELINUX */
@@ -1205,21 +1202,24 @@ static int send_and_receive(pam_handle_t *pamh, struct pam_items *pi,
                 goto done;
             }
 
-            pos = 0;
             len = strlen(pi->selinux_user);
-            while (pos < len) {
-                ret = write(fd, pi->selinux_user + pos, len-pos);
-                if (ret < 0) {
-                    if (errno != EINTR) {
-                        logger(pamh, LOG_ERR, "writing to SELinux data file "
-                               "failed. %s", tmp_path);
-                        pam_status = PAM_SYSTEM_ERR;
-                        goto done;
-                    }
-                    continue;
-                }
-                pos += ret;
+
+            errno = 0;
+            written = sss_atomic_write_s(fd, pi->selinux_user, len);
+            if (written == -1) {
+                ret = errno;
+                logger(pamh, LOG_ERR, "writing to SELinux data file "
+                        "failed. %s", tmp_path);
+                pam_status = PAM_SYSTEM_ERR;
+                goto done;
             }
+
+            if (written != len) {
+                logger(pamh, LOG_ERR, "Expected to write %d bytes, wrote %d",
+                       written, len);
+                goto done;
+            }
+
             close(fd);
 
             rename(tmp_path, path);
diff --git a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
index 6424d7b7c3c17375ad4ac8dc6ec2276786bf0560..e45ef45f10ddfe8c938265136928cb2b7999a989 100644
--- a/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
+++ b/src/sss_client/ssh/sss_ssh_knownhostsproxy.c
@@ -140,11 +140,11 @@ connect_socket(int family, struct sockaddr *addr, size_t addr_len)
                 }
 
                 errno = 0;
-                res = sss_atomic_write(i == 0 ? sock : 1, buffer, res);
+                res = sss_atomic_write_s(i == 0 ? sock : 1, buffer, res);
                 ret = errno;
                 if (res == -1) {
                     DEBUG(SSSDBG_OP_FAILURE,
-                          ("sss_atomic_write() failed (%d): %s\n",
+                          ("sss_atomic_write_s() failed (%d): %s\n",
                            ret, strerror(ret)));
                     goto done;
                 } else if (ret == EPIPE) {
diff --git a/src/tools/files.c b/src/tools/files.c
index 6947535492ea99709fdcaadb1e21e9cbc573629a..d80e3bd9a387f69cb7bf11dc2b09fe07a928911a 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -411,7 +411,7 @@ static int copy_file(const char *src,
     int ifd = -1;
     int ofd = -1;
     char buf[1024];
-    ssize_t cnt, written, res;
+    ssize_t cnt, written;
     struct stat fstatbuf;
 
     ifd = open(src, O_RDONLY);
@@ -463,41 +463,28 @@ static int copy_file(const char *src,
         goto fail;
     }
 
-    while ((cnt = read(ifd, buf, sizeof(buf))) != 0) {
+    while ((cnt = sss_atomic_read_s(ifd, buf, sizeof(buf))) != 0) {
         if (cnt == -1) {
-            if (errno == EINTR || errno == EAGAIN) {
-                continue;
-            }
-
-            DEBUG(1, ("Cannot read() from source file '%s': [%d][%s].\n",
-                        src, ret, strerror(ret)));
+            ret = errno;
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Cannot read() from source file '%s': [%d][%s].\n",
+                   src, ret, strerror(ret)));
             goto fail;
         }
-        else if (cnt > 0) {
-            /* Copy the buffer to the new file */
-            written = 0;
-            while (written < cnt) {
-                res = write(ofd, buf+written, (size_t)cnt-written);
-                if (res == -1) {
-                    ret = errno;
-                    if (ret == EINTR || ret == EAGAIN) {
-                        /* retry the write */
-                        continue;
-                    }
-                    DEBUG(1, ("Cannot write() to destination file '%s': [%d][%s].\n",
-                                dst, ret, strerror(ret)));
-                    goto fail;
-                }
-                else if (res <= 0) {
-                    DEBUG(1, ("Unexpected result from write(): [%d]\n", res));
-                    goto fail;
-                }
 
-                written += res;
-            }
+        errno = 0;
+        written = sss_atomic_write_s(ofd, buf, cnt);
+        if (written == -1) {
+            ret = errno;
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Cannot write() to destination file '%s': [%d][%s].\n",
+                   dst, ret, strerror(ret)));
+            goto fail;
         }
-        else {
-            DEBUG(1, ("Unexpected return code of read [%d]\n", cnt));
+
+        if (written != cnt) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Wrote %d bytes, expected %d\n", written, cnt));
             goto fail;
         }
     }
diff --git a/src/util/backup_file.c b/src/util/backup_file.c
index ba7764049d1e0866e7a0fb27177057fbd5198d94..edbf868cb8f68ca534c257a6dfc761dc9fd025f2 100644
--- a/src/util/backup_file.c
+++ b/src/util/backup_file.c
@@ -33,9 +33,8 @@ int backup_file(const char *src_file, int dbglvl)
     int src_fd = -1;
     int dst_fd = -1;
     char *dst_file;
-    ssize_t count;
-    ssize_t num;
-    ssize_t pos;
+    ssize_t numread;
+    ssize_t written;
     int ret, i;
 
     src_fd = open(src_file, O_RDONLY);
@@ -84,31 +83,30 @@ int backup_file(const char *src_file, int dbglvl)
 
     /* copy file contents */
     while (1) {
-        num = read(src_fd, buf, BUFFER_SIZE);
-        if (num < 0) {
-            if (errno == EINTR || errno == EAGAIN) continue;
+        errno = 0;
+        numread = sss_atomic_read_s(src_fd, buf, BUFFER_SIZE);
+        if (numread < 0) {
             ret = errno;
             DEBUG(dbglvl, ("Error (%d [%s]) reading from source %s\n",
                            ret, strerror(ret), src_file));
             goto done;
         }
-        if (num == 0) break;
+        if (numread == 0) break;
 
-        count = num;
+        errno = 0;
+        written = sss_atomic_write_s(dst_fd, buf, numread);
+        if (written == -1) {
+            ret = errno;
+            DEBUG(dbglvl, ("Error (%d [%s]) writing to destination %s\n",
+                            ret, strerror(ret), dst_file));
+            goto done;
+        }
 
-        pos = 0;
-        while (count > 0) {
-            errno = 0;
-            num = write(dst_fd, &buf[pos], count);
-            if (num < 0) {
-                if (errno == EINTR || errno == EAGAIN) continue;
-                ret = errno;
-                DEBUG(dbglvl, ("Error (%d [%s]) writing to destination %s\n",
-                               ret, strerror(ret), dst_file));
-                goto done;
-            }
-            pos += num;
-            count -= num;
+        if (written != numread) {
+            DEBUG(dbglvl, ("Wrote %d bytes expected %d bytes\n",
+                  written, numread));
+            ret = EIO;
+            goto done;
         }
     }
 
diff --git a/src/util/child_common.c b/src/util/child_common.c
index 6214c7cc39539d2dd9573ea9719bf9b48284c35f..05f00b0f3398c27e28f524595442fce4911e9e3c 100644
--- a/src/util/child_common.c
+++ b/src/util/child_common.c
@@ -337,42 +337,36 @@ static void write_pipe_handler(struct tevent_context *ev,
     struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
     struct write_pipe_state *state = tevent_req_data(req,
                                                      struct write_pipe_state);
-    ssize_t size;
+    errno_t ret;
 
     if (flags & TEVENT_FD_READ) {
-        DEBUG(1, ("write_pipe_done called with TEVENT_FD_READ,"
-                  " this should not happen.\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("write_pipe_done called with TEVENT_FD_READ,"
+               " this should not happen.\n"));
         tevent_req_error(req, EINVAL);
         return;
     }
 
-    size = write(state->fd,
-                 state->buf + state->written,
-                 state->len - state->written);
-    if (size == -1) {
-        if (errno == EAGAIN || errno == EINTR) return;
-        DEBUG(1, ("write failed [%d][%s].\n", errno, strerror(errno)));
-        tevent_req_error(req, errno);
-        return;
-
-    } else if (size >= 0) {
-        state->written += size;
-        if (state->written > state->len) {
-            DEBUG(1, ("write to much, this should never happen.\n"));
-            tevent_req_error(req, EINVAL);
-            return;
-        }
-    } else {
-        DEBUG(1, ("unexpected return value of write [%d].\n", size));
-        tevent_req_error(req, EINVAL);
+    errno = 0;
+    state->written = sss_atomic_write_s(state->fd, state->buf, state->len);
+    if (state->written == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("write failed [%d][%s].\n", ret, strerror(ret)));
+        tevent_req_error(req, ret);
         return;
     }
 
-    if (state->len == state->written) {
-        DEBUG(6, ("All data has been sent!\n"));
-        tevent_req_done(req);
+    if (state->len != state->written) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Wrote %d bytes, expected %d\n",
+              state->written, state->len));
+        tevent_req_error(req, EIO);
         return;
     }
+
+    DEBUG(SSSDBG_TRACE_FUNC, ("All data has been sent!\n"));
+    tevent_req_done(req);
+    return;
 }
 
 int write_pipe_recv(struct tevent_req *req)
@@ -438,16 +432,13 @@ static void read_pipe_handler(struct tevent_context *ev,
         return;
     }
 
-    size = read(state->fd,
+    size = sss_atomic_read_s(state->fd,
                 buf,
                 CHILD_MSG_CHUNK);
     if (size == -1) {
         err = errno;
-        if (err == EAGAIN || err == EINTR) {
-            return;
-        }
-
-        DEBUG(1, ("read failed [%d][%s].\n", err, strerror(err)));
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("read failed [%d][%s].\n", err, strerror(err)));
         tevent_req_error(req, err);
         return;
 
diff --git a/src/util/find_uid.c b/src/util/find_uid.c
index 77b9f22a83089ad44e6bda75615cad55b49ff8f4..d34a4abd2c2bb4965ff49b9a750f387519544b0e 100644
--- a/src/util/find_uid.c
+++ b/src/util/find_uid.c
@@ -107,15 +107,13 @@ static errno_t get_uid_from_pid(const pid_t pid, uid_t *uid)
         goto fail_fd;
     }
 
-    while ((ret = read(fd, buf, BUFSIZE)) != 0) {
-        if (ret == -1) {
-            error = errno;
-            if (error == EINTR || error == EAGAIN) {
-                continue;
-            }
-            DEBUG(1, ("read failed [%d][%s].\n", error, strerror(error)));
-            goto fail_fd;
-        }
+    errno = 0;
+    ret = sss_atomic_read_s(fd, buf, BUFSIZE);
+    if (ret == -1) {
+        error = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("read failed [%d][%s].\n", error, strerror(error)));
+        goto fail_fd;
     }
 
     /* Guarantee NULL-termination in case we read the full BUFSIZE somehow */
diff --git a/src/util/server.c b/src/util/server.c
index 95b183632f46852867fb2a36c9517af12fb741d1..39cc1277681f88dd1c3dd5a8ed95ba3df3cc10b3 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -117,51 +117,36 @@ int pidfile(const char *path, const char *name)
     fd = open(file, O_RDONLY, 0644);
     err = errno;
     if (fd != -1) {
-        len = 0;
-        while ((ret = read(fd, pid_str + len, pidlen - len)) != 0) {
-            if (ret == -1) {
-                if (errno == EINTR || errno == EAGAIN) {
-                    continue;
-                }
-                DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno)));
-                break;
-            } else if (ret > 0) {
-                len += ret;
-                if (len > pidlen) {
-                    DEBUG(1, ("read too much, this should never happen.\n"));
-                    close(fd);
-                    talloc_free(file);
-                    return EINVAL;
-                }
-                continue;
-            } else {
-                DEBUG(1, ("unexpected return code of read [%d].\n", ret));
-                break;
-            }
+        errno = 0;
+        len = sss_atomic_read_s(fd, pid_str, pidlen);
+        ret = errno;
+        if (len == -1) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("read failed [%d][%s].\n", ret, strerror(ret)));
+            close(fd);
+            talloc_free(file);
+            return EINVAL;
         }
 
         /* Ensure NULL-termination */
         pid_str[len] = '\0';
 
-        if (ret == 0) {
-            /* let's check the pid */
-
-            pid = (pid_t)atoi(pid_str);
-            if (pid != 0) {
-                errno = 0;
-                ret = kill(pid, 0);
-                /* succeeded in signaling the process -> another sssd process */
-                if (ret == 0) {
-                    close(fd);
-                    talloc_free(file);
-                    return EEXIST;
-                }
-                if (ret != 0 && errno != ESRCH) {
-                    err = errno;
-                    close(fd);
-                    talloc_free(file);
-                    return err;
-                }
+        /* let's check the pid */
+        pid = (pid_t)atoi(pid_str);
+        if (pid != 0) {
+            errno = 0;
+            ret = kill(pid, 0);
+            /* succeeded in signaling the process -> another sssd process */
+            if (ret == 0) {
+                close(fd);
+                talloc_free(file);
+                return EEXIST;
+            }
+            if (ret != 0 && errno != ESRCH) {
+                err = errno;
+                close(fd);
+                talloc_free(file);
+                return err;
             }
         }
 
@@ -188,25 +173,20 @@ int pidfile(const char *path, const char *name)
     snprintf(pid_str, sizeof(pid_str) -1, "%u\n", (unsigned int) getpid());
     size = strlen(pid_str);
 
-    written = 0;
-    while (written < size) {
-        ret = write(fd, pid_str+written, size-written);
-        if (ret == -1) {
-            err = errno;
-            if (err == EINTR || err == EAGAIN) {
-                continue;
-            }
-            DEBUG(1, ("write failed [%d][%s]\n", err, strerror(err)));
-            break;
-        }
-        else {
-            written += ret;
-        }
+    errno = 0;
+    written = sss_atomic_write_s(fd, pid_str, size);
+    if (ret == -1) {
+        err = errno;
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("write failed [%d][%s]\n", err, strerror(err)));
+        return err;
     }
 
     if (written != size) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Wrote %d bytes expected %d\n", written, size));
         close(fd);
-        return err;
+        return EIO;
     }
 
     close(fd);
@@ -317,8 +297,10 @@ static void server_stdin_handler(struct tevent_context *event_ctx,
 {
 	const char *binary_name = (const char *)private;
 	uint8_t c;
-	if (read(0, &c, 1) == 0) {
-		DEBUG(0,("%s: EOF on stdin - terminating\n", binary_name));
+
+        errno = 0;
+        if (sss_atomic_read_s(0, &c, 1) == 0) {
+		DEBUG(SSSDBG_CRIT_FAILURE,("%s: EOF on stdin - terminating\n", binary_name));
 #if HAVE_GETPGRP
 		if (getpgrp() == getpid()) {
 			kill(-getpgrp(), SIGTERM);
-- 
1.7.7.6



More information about the sssd-devel mailing list