[SSSD] [PATCH] GPO: Use sss_unique_file and close fd on failure

Jakub Hrozek jhrozek at redhat.com
Tue Sep 1 09:54:35 UTC 2015


On Tue, Sep 01, 2015 at 11:28:32AM +0200, Pavel Březina wrote:
> >Please see the attached patch, does it look better to you?
> 
> Hi, yes. There is just an indentation error:
> 
> >+    if (fd != -1) {
> >+        fret = close(fd);
> >+        if (fret == -1) {
> >+            fret = errno;
> >+            DEBUG(SSSDBG_CRIT_FAILURE,
> >+                "close failed [%d][%s].\n", fret, strerror(fret));
> 
>                    ^^
> 
> >+        }

Let's try it again :-)
-------------- next part --------------
>From 600f4aeb12d7371e86e1d03ca68057004538ff50 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 12 Aug 2015 12:45:34 +0200
Subject: [PATCH] GPO: Use sss_unique_file and close fd on failure

The GPO child didn't remove temporary file on failure and didn't close
the fd on failure (the latter was not much of a problem for a
short-lived child process).
---
 src/providers/ad/ad_gpo_child.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/providers/ad/ad_gpo_child.c b/src/providers/ad/ad_gpo_child.c
index 6547f9c05ea53134548ebd7f998fd66b25832854..1b599716487793524e35ebb1b7b7626539b18bb9 100644
--- a/src/providers/ad/ad_gpo_child.c
+++ b/src/providers/ad/ad_gpo_child.c
@@ -273,10 +273,10 @@ static errno_t gpo_cache_store_file(const char *smb_path,
                                     int buflen)
 {
     int ret;
+    int fret;
     int fd = -1;
     char *tmp_name = NULL;
     ssize_t written;
-    mode_t old_umask;
     char *filename = NULL;
     char *smb_path_with_suffix = NULL;
     TALLOC_CTX *tmp_ctx = NULL;
@@ -318,13 +318,10 @@ static errno_t gpo_cache_store_file(const char *smb_path,
         goto done;
     }
 
-    old_umask = umask(077);
-    fd = mkstemp(tmp_name);
-    umask(old_umask);
+    fd = sss_unique_file(tmp_ctx, tmp_name, &ret);
     if (fd == -1) {
-        ret = errno;
         DEBUG(SSSDBG_CRIT_FAILURE,
-              "mkstemp failed [%d][%s].\n", ret, strerror(ret));
+              "sss_unique_file failed [%d][%s].\n", ret, strerror(ret));
         goto done;
     }
 
@@ -353,14 +350,6 @@ static errno_t gpo_cache_store_file(const char *smb_path,
         goto done;
     }
 
-    ret = close(fd);
-    if (ret == -1) {
-        ret = errno;
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "close failed [%d][%s].\n", ret, strerror(ret));
-        goto done;
-    }
-
     ret = rename(tmp_name, filename);
     if (ret == -1) {
         ret = errno;
@@ -369,10 +358,19 @@ static errno_t gpo_cache_store_file(const char *smb_path,
         goto done;
     }
 
+    ret = EOK;
  done:
-
     if (ret != EOK) {
-      DEBUG(SSSDBG_CRIT_FAILURE, "Error encountered: %d.\n", ret);
+        DEBUG(SSSDBG_CRIT_FAILURE, "Error encountered: %d.\n", ret);
+    }
+
+    if (fd != -1) {
+        fret = close(fd);
+        if (fret == -1) {
+            fret = errno;
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "close failed [%d][%s].\n", fret, strerror(fret));
+        }
     }
 
     talloc_free(tmp_ctx);
-- 
2.4.3



More information about the sssd-devel mailing list