[SSSD] [PATCH] LDAP: avoid complex realloc logic in save_rfc2307bis_group_memberships

Jakub Hrozek jhrozek at redhat.com
Tue Jan 15 16:38:01 UTC 2013


The function tried to be smart and realloc only when needed, but that
only lead to hard-to find bugs where the logic would not allocate the
proper space. Remove the reallocation and prefer readability over speed
in this case.

In particular, if one iteration hit the "if (num_added == 0) add=NULL"
condition but the next group being processed had no members and set
grp_count=0, then we'd use the add pointer from last iteration that was
still NULL.
-------------- next part --------------
>From f0ea94133d9bd21364afb8362bcf2562f9473224 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 15 Jan 2013 17:31:18 +0100
Subject: [PATCH] LDAP: avoid complex realloc logic in
 save_rfc2307bis_group_memberships

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

The function tried to be smart and realloc only when needed, but that
only lead to hard-to find bugs where the logic would not allocate the
proper space. Remove the reallocation and prefer readability over speed
in this case.
---
 src/providers/ldap/sdap_async_initgroups.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index 698c9e1c21bbde646c355903281890fea0b4d819..8c0b0167af836b1b0a4ff6b066e722c32e1474ce 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -1803,7 +1803,6 @@ save_rfc2307bis_group_memberships(struct sdap_initgr_rfc2307bis_state *state)
     int num_added;
     int i;
     int grp_count;
-    int grp_count_old = 0;
     char **add = NULL;
 
     tmp_ctx = talloc_new(NULL);
@@ -1843,12 +1842,10 @@ save_rfc2307bis_group_memberships(struct sdap_initgr_rfc2307bis_state *state)
          * nesting limit. This array must be NULL terminated.
          */
         for (grp_count = 0; iter->add[grp_count]; grp_count++);
-        if (grp_count > grp_count_old) {
-            add = talloc_realloc(tmp_ctx, add, char*, grp_count + 1);
-            if (add == NULL) {
-                ret = ENOMEM;
-                goto done;
-            }
+        add = talloc_zero_array(tmp_ctx, char *, grp_count + 1);
+        if (add == NULL) {
+            ret = ENOMEM;
+            goto done;
         }
 
         num_added = 0;
@@ -1862,11 +1859,6 @@ save_rfc2307bis_group_memberships(struct sdap_initgr_rfc2307bis_state *state)
             }
         }
 
-        /* Swap old and new group counter. */
-        grp_count ^= grp_count_old;
-        grp_count_old ^= grp_count;
-        grp_count ^= grp_count_old;
-
         if (num_added == 0) {
             add = NULL;
         } else {
-- 
1.8.0.2



More information about the sssd-devel mailing list