[SSSD] [PATCH] LDAP: Always fail if a map can't be found

Jakub Hrozek jhrozek at redhat.com
Mon Apr 8 15:33:09 UTC 2013


On Fri, Apr 05, 2013 at 11:19:31PM +0200, Lukas Slebodnik wrote:
> On (05/04/13 17:39), Jakub Hrozek wrote:
> >Hopefully this is the last real issue that Coverity discovered.
> 
> >From 4ec51e8f8ccfee414a8ce01a093ca929c0835ea2 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Fri, 5 Apr 2013 15:14:34 +0200
> >Subject: [PATCH] LDAP: Always fail if a map can't be found
> >
> >---
> > src/providers/ldap/sdap.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> >diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
> >index dba4e41db0039632939f275f9295321afe7a31ae..5cdbffd92d5b5eb9fa3212b324ad91a2da6c2120 100644
> >--- a/src/providers/ldap/sdap.c
> >+++ b/src/providers/ldap/sdap.c
> >@@ -77,10 +77,7 @@ int sdap_get_map(TALLOC_CTX *memctx,
> >         if (map[i].def_name && !map[i].name) {
> >             DEBUG(SSSDBG_CRIT_FAILURE,
> >                   ("Failed to retrieve value for %s\n", map[i].opt_name));
> >-            if (ret != EOK) {
> >-                talloc_zfree(map);
>                    ^^^^^
> I think, that talloc_zfree should not be removed.

Correct, thank you.

> >-                return EINVAL;
> >-            }
> >+            return EINVAL;
> >         }
> > 
> >         DEBUG(SSSDBG_TRACE_FUNC, ("Option %s has%s value %s\n",
> >-- 
> >
> 
> I think that whole function should be rewritten, because there is a lot of
> duplicated cleanup code (talloc_zfree && return EINVAL)

If you feel strongly about this, file a ticket. We don't have the cycles
to refactor in 1.10 beta anymore.

A new patch is attached.
-------------- next part --------------
>From b32ab72a7a1d43dc895993605d7ed82758b6fce1 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 5 Apr 2013 15:14:34 +0200
Subject: [PATCH] LDAP: Always fail if a map can't be found

---
 src/providers/ldap/sdap.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index dba4e41db0039632939f275f9295321afe7a31ae..288d2d594a1401e9dbcc40b92cb68e946322791e 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -77,10 +77,8 @@ int sdap_get_map(TALLOC_CTX *memctx,
         if (map[i].def_name && !map[i].name) {
             DEBUG(SSSDBG_CRIT_FAILURE,
                   ("Failed to retrieve value for %s\n", map[i].opt_name));
-            if (ret != EOK) {
-                talloc_zfree(map);
-                return EINVAL;
-            }
+            talloc_zfree(map);
+            return EINVAL;
         }
 
         DEBUG(SSSDBG_TRACE_FUNC, ("Option %s has%s value %s\n",
-- 
1.8.1.4



More information about the sssd-devel mailing list