On 10/12/12 10:38, Jakub Hrozek wrote:
On Fri, Dec 07, 2012 at 08:59:40PM +0100, Ondrej Kos wrote:
https://fedorahosted.org/sssd/ticket/1685

when trying to delete credentials not present in cache, the return
value was misread, taking ENOENT as a failed part of code.

Fixing patch is attached

Ondra

Looks good, just one question:

+static int
+delete_user(struct sysdb_ctx *sysdb, const char *name, uid_t uid)
+{
+    int ret = EOK;
+
+    DEBUG(SSSDBG_TRACE_FUNC,
+          ("User %s does not exist (or is invalid) on remote server,"
+           " deleting!\n", name));
+    ret = sysdb_delete_user(sysdb, name, uid);
+    if (ret == ENOENT) {
+        ret = EOK;
+        errno = EOK;

Why set errno? We don't use it (I hope) to check the results of delete
user..

Good catch, I was firstly confused by the error message produced:
[sssd[nss]] [sss_dp_get_reply] (0x1000): Got reply from Data Provider - DP error code: 3 errno: 2 error

so i originally for test reset both return value and errno, but looking at the code now, we don't use errno.

New patch is attached.


+    }
+
+    return ret;
+}
+

The rest looks good. I haven't tested the patch yet.
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



--
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech

phone: +420-532-294-558
cell:  +420-736-417-909
ext:   82-62558
loc:   1013 Brno 1 office
irc:   okos @ #brno
From 49c4325ce75ed9c039176fe1ce7be696c94626e4 Mon Sep 17 00:00:00 2001
From: Ondrej Kos <[email protected]>
Date: Fri, 7 Dec 2012 20:44:15 +0100
Subject: [PATCH] PROXY: fix negative cache

https://fedorahosted.org/sssd/ticket/1685

The PROXY provider wasn't storing credentials to negative cache due to
bad return value. This was delegated from attempt to delete these
credentials from local cache. Therefore ENOENT is replaced as EOK.
---
 src/providers/proxy/proxy_id.c | 44 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index 87eb91b1ee47fce61757aa58efc13d84ecc2acd2..060c4723bdb4c2ad6e671957b90b3e3af0551878 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -35,6 +35,9 @@ static int
 handle_getpw_result(enum nss_status status, struct passwd *pwd,
                     struct sss_domain_info *dom, bool *del_user);
 
+static int
+delete_user(struct sysdb_ctx *sysdb, const char *name, uid_t uid);
+
 static int get_pw_name(TALLOC_CTX *mem_ctx,
                        struct proxy_id_ctx *ctx,
                        struct sysdb_ctx *sysdb,
@@ -83,10 +86,7 @@ static int get_pw_name(TALLOC_CTX *mem_ctx,
     }
 
     if (del_user) {
-        DEBUG(SSSDBG_TRACE_FUNC,
-              ("User %s does not exist (or is invalid) on remote server,"
-               " deleting!\n", name));
-        ret = sysdb_delete_user(sysdb, name, 0);
+        ret = delete_user(sysdb, name, 0);
         goto done;
     }
 
@@ -126,10 +126,7 @@ static int get_pw_name(TALLOC_CTX *mem_ctx,
     }
 
     if (del_user) {
-        DEBUG(SSSDBG_TRACE_FUNC,
-              ("User %s does not exist (or is invalid) on remote server,"
-               " deleting!\n", name));
-        ret = sysdb_delete_user(sysdb, name, uid);
+        ret = delete_user(sysdb, name, uid);
         goto done;
     }
 
@@ -197,6 +194,22 @@ handle_getpw_result(enum nss_status status, struct passwd *pwd,
     return ret;
 }
 
+static int
+delete_user(struct sysdb_ctx *sysdb, const char *name, uid_t uid)
+{
+    int ret = EOK;
+
+    DEBUG(SSSDBG_TRACE_FUNC,
+          ("User %s does not exist (or is invalid) on remote server,"
+           " deleting!\n", name));
+    ret = sysdb_delete_user(sysdb, name, uid);
+    if (ret == ENOENT) {
+        ret = EOK;
+    }
+
+    return ret;
+}
+
 static int save_user(struct sysdb_ctx *sysdb, bool lowercase,
                      struct passwd *pwd, const char *real_name,
                      const char *alias, uint64_t cache_timeout)
@@ -319,10 +332,7 @@ static int get_pw_uid(TALLOC_CTX *mem_ctx,
     }
 
     if (del_user) {
-        DEBUG(SSSDBG_TRACE_FUNC,
-              ("User %d does not exist (or is invalid) on remote server,"
-               " deleting!\n", uid));
-        ret = sysdb_delete_user(sysdb, NULL, uid);
+        ret = delete_user(sysdb, NULL, uid);
         goto done;
     }
 
@@ -1154,10 +1164,7 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
     }
 
     if (del_user) {
-        DEBUG(SSSDBG_TRACE_FUNC,
-              ("User %s does not exist (or is invalid) on remote server,"
-               " deleting!\n", name));
-        ret = sysdb_delete_user(sysdb, name, 0);
+        ret = delete_user(sysdb, name, 0);
         if (ret) {
             DEBUG(SSSDBG_OP_FAILURE, ("Could not delete user\n"));
             goto fail;
@@ -1201,10 +1208,7 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
     }
 
     if (del_user) {
-        DEBUG(SSSDBG_TRACE_FUNC,
-              ("User %s does not exist (or is invalid) on remote server,"
-               " deleting!\n", name));
-        ret = sysdb_delete_user(sysdb, name, uid);
+        ret = delete_user(sysdb, name, uid);
         if (ret) {
             DEBUG(SSSDBG_OP_FAILURE, ("Could not delete user\n"));
             goto fail;
-- 
1.7.11.7

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to