Hopefully this is the last real issue that Coverity discovered.
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@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.
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)
LS
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@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.
On (08/04/13 17:33), Jakub Hrozek wrote:
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@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.
This was jaust a alternative. Your patch is also good solution.
A new patch is attached.
Ack
LS
On Tue, Apr 09, 2013 at 09:00:26AM +0200, Lukas Slebodnik wrote:
On (08/04/13 17:33), Jakub Hrozek wrote:
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@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.
This was jaust a alternative. Your patch is also good solution.
A new patch is attached.
Ack
LS
Pushed to master.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik