389-ds-base

Clone
Source Code
GIT

#51085 Issue 51076 - remove unnecessary slapi entry dups
Closed 3 years ago by spichugi. Opened 3 years ago by mreynolds.
mreynolds/389-ds-base issue51076  into  master

file modified
+3 -3 
@@ -37,6 +37,7 @@ 
 

  int
 

  acct_policy_load_config_startup(Slapi_PBlock *pb __attribute__((unused)), void *plugin_id)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      acctPluginCfg *newcfg;
 

      Slapi_Entry *config_entry = NULL;
 

      Slapi_DN *config_sdn = NULL;
 
@@ -44,8 +45,7 @@ 
 

  

      /* Retrieve the config entry */
 

      config_sdn = slapi_sdn_new_normdn_byref(PLUGIN_CONFIG_DN);
 

-     rc = slapi_search_internal_get_entry(config_sdn, NULL, &config_entry,
 

-                                          plugin_id);
 

+     rc = slapi_search_get_entry(&entry_pb, config_sdn, NULL, &config_entry, plugin_id);
 

      slapi_sdn_free(&config_sdn);
 

  

      if (rc != LDAP_SUCCESS || config_entry == NULL) {
 
@@ -60,7 +60,7 @@ 
 

      rc = acct_policy_entry2config(config_entry, newcfg);
 

      config_unlock();
 

  

-     slapi_entry_free(config_entry);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      return (rc);
 

  }

file modified
+16 -20 
@@ -209,6 +209,7 @@ 
 

  int
 

  acct_bind_preop(Slapi_PBlock *pb)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      const char *dn = NULL;
 

      Slapi_DN *sdn = NULL;
 

      Slapi_Entry *target_entry = NULL;
 
@@ -236,8 +237,7 @@ 
 

          goto done;
 

      }
 

  

-     ldrc = slapi_search_internal_get_entry(sdn, NULL, &target_entry,
 

-                                            plugin_id);
 

+     ldrc = slapi_search_get_entry(&entry_pb, sdn, NULL, &target_entry, plugin_id);
 

  

      /* There was a problem retrieving the entry */
 

      if (ldrc != LDAP_SUCCESS) {
 
@@ -275,7 +275,7 @@ 
 

          slapi_send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL, NULL, 0, NULL);
 

      }
 

  

-     slapi_entry_free(target_entry);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      free_acctpolicy(&policy);
 

  
@@ -293,6 +293,7 @@ 
 

  int
 

  acct_bind_postop(Slapi_PBlock *pb)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      char *dn = NULL;
 

      int ldrc, tracklogin = 0;
 

      int rc = 0; /* Optimistic default */
 
@@ -327,8 +328,7 @@ 
 

         covered by an account policy to decide whether we should track */
 

      if (tracklogin == 0) {
 

          sdn = slapi_sdn_new_normdn_byref(dn);
 

-         ldrc = slapi_search_internal_get_entry(sdn, NULL, &target_entry,
 

-                                                plugin_id);
 

+         ldrc = slapi_search_get_entry(&entry_pb, sdn, NULL, &target_entry, plugin_id);
 

  

          if (ldrc != LDAP_SUCCESS) {
 

              slapi_log_err(SLAPI_LOG_ERR, POST_PLUGIN_NAME,
 
@@ -355,7 +355,7 @@ 
 

          slapi_send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL, NULL, 0, NULL);
 

      }
 

  

-     slapi_entry_free(target_entry);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      slapi_sdn_free(&sdn);
 

  
@@ -370,11 +370,11 @@ 
 

  static int
 

  acct_pre_op(Slapi_PBlock *pb, int modop)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      Slapi_DN *sdn = 0;
 

      Slapi_Entry *e = 0;
 

      Slapi_Mods *smods = 0;
 

      LDAPMod **mods;
 

-     int free_entry = 0;
 

      char *errstr = NULL;
 

      int ret = SLAPI_PLUGIN_SUCCESS;
 

  
@@ -384,28 +384,25 @@ 
 

  

      if (acct_policy_dn_is_config(sdn)) {
 

          /* Validate config changes, but don't apply them.
 

-      * This allows us to reject invalid config changes
 

-      * here at the pre-op stage.  Applying the config
 

-      * needs to be done at the post-op stage. */
 

+          * This allows us to reject invalid config changes
 

+          * here at the pre-op stage.  Applying the config
 

+          * needs to be done at the post-op stage. */
 

  

          if (LDAP_CHANGETYPE_ADD == modop) {
 

              slapi_pblock_get(pb, SLAPI_ADD_ENTRY, &e);
 

  

-             /* If the entry doesn't exist, just bail and
 

-      * let the server handle it. */
 

+             /* If the entry doesn't exist, just bail and let the server handle it. */
 

              if (e == NULL) {
 

                  goto bail;
 

              }
 

          } else if (LDAP_CHANGETYPE_MODIFY == modop) {
 

              /* Fetch the entry being modified so we can
 

-      * create the resulting entry for validation. */
 

+              * create the resulting entry for validation. */
 

              if (sdn) {
 

-                 slapi_search_internal_get_entry(sdn, 0, &e, get_identity());
 

-                 free_entry = 1;
 

+                 slapi_search_get_entry(&entry_pb, sdn, 0, &e, get_identity());
 

              }
 

  

-             /* If the entry doesn't exist, just bail and
 

-      * let the server handle it. */
 

+             /* If the entry doesn't exist, just bail and let the server handle it. */
 

              if (e == NULL) {
 

                  goto bail;
 

              }
 
@@ -418,7 +415,7 @@ 
 

              /* Apply the  mods to create the resulting entry. */
 

              if (mods && (slapi_entry_apply_mods(e, mods) != LDAP_SUCCESS)) {
 

                  /* The mods don't apply cleanly, so we just let this op go
 

-      * to let the main server handle it. */
 

+                  * to let the main server handle it. */
 

                  goto bailmod;
 

              }
 

          } else if (modop == LDAP_CHANGETYPE_DELETE) {
 
@@ -439,8 +436,7 @@ 
 

      }
 

  

  bail:
 

-     if (free_entry && e)
 

-         slapi_entry_free(e);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      if (ret) {
 

          slapi_log_err(SLAPI_LOG_PLUGIN, PRE_PLUGIN_NAME,

file modified
+3 -3 
@@ -85,6 +85,7 @@ 
 

  int
 

  get_acctpolicy(Slapi_PBlock *pb __attribute__((unused)), Slapi_Entry *target_entry, void *plugin_id, acctPolicy **policy)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      Slapi_DN *sdn = NULL;
 

      Slapi_Entry *policy_entry = NULL;
 

      Slapi_Attr *attr;
 
@@ -123,8 +124,7 @@ 
 

      }
 

  

      sdn = slapi_sdn_new_dn_byref(policy_dn);
 

-     ldrc = slapi_search_internal_get_entry(sdn, NULL, &policy_entry,
 

-                                            plugin_id);
 

+     ldrc = slapi_search_get_entry(&entry_pb, sdn, NULL, &policy_entry, plugin_id);
 

      slapi_sdn_free(&sdn);
 

  

      /* There should be a policy but it can't be retrieved; fatal error */
 
@@ -160,7 +160,7 @@ 
 

  done:
 

      config_unlock();
 

      slapi_ch_free_string(&policy_dn);
 

-     slapi_entry_free(policy_entry);
 

+     slapi_search_get_entry_done(&entry_pb);
 

      return (rc);
 

  }

file modified
+6 -11 
@@ -1629,13 +1629,12 @@ 
 

      char *member_value = NULL;
 

      int rc = 0;
 

      Slapi_DN *group_sdn;
 

-     Slapi_Entry *group_entry = NULL;
 

  

      /* First thing check that the group still exists */
 

      group_sdn = slapi_sdn_new_dn_byval(group_dn);
 

-     rc = slapi_search_internal_get_entry(group_sdn, NULL, &group_entry, automember_get_plugin_id());
 

+     rc = slapi_search_internal_get_entry(group_sdn, NULL, NULL, automember_get_plugin_id());
 

      slapi_sdn_free(&group_sdn);
 

-     if (rc != LDAP_SUCCESS || group_entry == NULL) {
 

+     if (rc != LDAP_SUCCESS) {
 

          if (rc == LDAP_NO_SUCH_OBJECT) {
 

              /* the automember group (default or target) does not exist, just skip this definition */
 

              slapi_log_err(SLAPI_LOG_INFO, AUTOMEMBER_PLUGIN_SUBSYSTEM,
 
@@ -1647,10 +1646,8 @@ 
 

                        "automember_update_member_value - group (default or target) can not be retrieved (%s) err=%d\n",
 

                        group_dn, rc);
 

          }
 

-         slapi_entry_free(group_entry);
 

          return rc;
 

      }
 

-     slapi_entry_free(group_entry);
 

  

      /* If grouping_value is dn, we need to fetch the dn instead. */
 

      if (slapi_attr_type_cmp(grouping_value, "dn", SLAPI_TYPE_CMP_EXACT) == 0) {
 
@@ -1752,11 +1749,11 @@ 
 

  static int
 

  automember_pre_op(Slapi_PBlock *pb, int modop)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      Slapi_DN *sdn = 0;
 

      Slapi_Entry *e = 0;
 

      Slapi_Mods *smods = 0;
 

      LDAPMod **mods;
 

-     int free_entry = 0;
 

      char *errstr = NULL;
 

      int ret = SLAPI_PLUGIN_SUCCESS;
 

  
@@ -1784,8 +1781,7 @@ 
 

              /* Fetch the entry being modified so we can
 

               * create the resulting entry for validation. */
 

              if (sdn) {
 

-                 slapi_search_internal_get_entry(sdn, 0, &e, automember_get_plugin_id());
 

-                 free_entry = 1;
 

+                 slapi_search_get_entry(&entry_pb, sdn, 0, &e, automember_get_plugin_id());
 

              }
 

  

              /* If the entry doesn't exist, just bail and
 
@@ -1799,7 +1795,7 @@ 
 

              smods = slapi_mods_new();
 

              slapi_mods_init_byref(smods, mods);
 

  

-             /* Apply the  mods to create the resulting entry. */
 

+             /* Apply the mods to create the resulting entry. */
 

              if (mods && (slapi_entry_apply_mods(e, mods) != LDAP_SUCCESS)) {
 

                  /* The mods don't apply cleanly, so we just let this op go
 

                   * to let the main server handle it. */
 
@@ -1831,8 +1827,7 @@ 
 

      }
 

  

  bail:
 

-     if (free_entry && e)
 

-         slapi_entry_free(e);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      if (ret) {
 

          slapi_log_err(SLAPI_LOG_PLUGIN, AUTOMEMBER_PLUGIN_SUBSYSTEM,

file modified
+9 -14 
@@ -1178,7 +1178,6 @@ 
 

  

      value = slapi_entry_attr_get_charptr(e, DNA_SHARED_CFG_DN);
 

      if (value) {
 

-         Slapi_Entry *shared_e = NULL;
 

          Slapi_DN *sdn = NULL;
 

          char *normdn = NULL;
 

          char *attrs[2];
 
@@ -1197,10 +1196,8 @@ 
 

          /* We don't need attributes */
 

          attrs[0] = "cn";
 

          attrs[1] = NULL;
 

-         slapi_search_internal_get_entry(sdn, attrs, &shared_e, getPluginID());
 

- 

          /* Make sure that the shared config entry exists. */
 

-         if (!shared_e) {
 

+         if(slapi_search_internal_get_entry(sdn, attrs, NULL, getPluginID()) != LDAP_SUCCESS) {
 

              /* We didn't locate the shared config container entry. Log
 

               * a message and skip this config entry. */
 

              slapi_log_err(SLAPI_LOG_ERR, DNA_PLUGIN_SUBSYSTEM,
 
@@ -1210,9 +1207,6 @@ 
 

              ret = DNA_FAILURE;
 

              slapi_sdn_free(&sdn);
 

              goto bail;
 

-         } else {
 

-             slapi_entry_free(shared_e);
 

-             shared_e = NULL;
 

          }
 

  

          normdn = (char *)slapi_sdn_get_dn(sdn);
 
@@ -1539,6 +1533,7 @@ 
 

  static int
 

  dna_load_host_port(void)
 

  {
 

+     Slapi_PBlock *pb = NULL;
 

      int status = DNA_SUCCESS;
 

      Slapi_Entry *e = NULL;
 

      Slapi_DN *config_dn = NULL;
 
@@ -1554,7 +1549,7 @@ 
 

  

      config_dn = slapi_sdn_new_ndn_byref("cn=config");
 

      if (config_dn) {
 

-         slapi_search_internal_get_entry(config_dn, attrs, &e, getPluginID());
 

+         slapi_search_get_entry(&pb, config_dn, attrs, &e, getPluginID());
 

          slapi_sdn_free(&config_dn);
 

      }
 

  
@@ -1562,8 +1557,8 @@ 
 

          hostname = slapi_entry_attr_get_charptr(e, "nsslapd-localhost");
 

          portnum = slapi_entry_attr_get_charptr(e, "nsslapd-port");
 

          secureportnum = slapi_entry_attr_get_charptr(e, "nsslapd-secureport");
 

-         slapi_entry_free(e);
 

      }
 

+     slapi_search_get_entry_done(&pb);
 

  

      if (!hostname || !portnum) {
 

          status = DNA_FAILURE;
 
@@ -2876,6 +2871,7 @@ 
 

  static int
 

  dna_is_replica_bind_dn(char *range_dn, char *bind_dn)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      char *replica_dn = NULL;
 

      Slapi_DN *replica_sdn = NULL;
 

      Slapi_DN *range_sdn = NULL;
 
@@ -2912,8 +2908,7 @@ 
 

          attrs[2] = 0;
 

  

          /* Find cn=replica entry via search */
 

-         slapi_search_internal_get_entry(replica_sdn, attrs, &e, getPluginID());
 

- 

+         slapi_search_get_entry(&entry_pb, replica_sdn, attrs, &e, getPluginID());
 

          if (e) {
 

              /* Check if the passed in bind dn matches any of the replica bind dns. */
 

              Slapi_Value *bind_dn_sv = slapi_value_new_string(bind_dn);
 
@@ -2927,6 +2922,7 @@ 
 

                  attrs[0] = "member";
 

                  attrs[1] = "uniquemember";
 

                  attrs[2] = 0;
 

+                 slapi_search_get_entry_done(&entry_pb);
 

                  for (i = 0; bind_group_dn != NULL && bind_group_dn[i] != NULL; i++) {
 

                      if (ret) {
 

                          /* already found a member, just free group */
 
@@ -2934,14 +2930,14 @@ 
 

                          continue;
 

                      }
 

                      bind_group_sdn = slapi_sdn_new_normdn_passin(bind_group_dn[i]);
 

-                     slapi_search_internal_get_entry(bind_group_sdn, attrs, &bind_group_entry, getPluginID());
 

+                     slapi_search_get_entry(&entry_pb, bind_group_sdn, attrs, &bind_group_entry, getPluginID());
 

                      if (bind_group_entry) {
 

                          ret = slapi_entry_attr_has_syntax_value(bind_group_entry, "member", bind_dn_sv);
 

                          if (ret == 0) {
 

                              ret = slapi_entry_attr_has_syntax_value(bind_group_entry, "uniquemember", bind_dn_sv);
 

                          }
 

                      }
 

-                     slapi_entry_free(bind_group_entry);
 

+                     slapi_search_get_entry_done(&entry_pb);
 

                      slapi_sdn_free(&bind_group_sdn);
 

                  }
 

                  slapi_ch_free((void **)&bind_group_dn);
 
@@ -2956,7 +2952,6 @@ 
 

      }
 

  

  done:
 

-     slapi_entry_free(e);
 

      slapi_sdn_free(&range_sdn);
 

      slapi_sdn_free(&replica_sdn);

file modified
+8 -8 
@@ -884,7 +884,7 @@ 
 

              pre_sdn = slapi_entry_get_sdn(pre_e);
 

              post_sdn = slapi_entry_get_sdn(post_e);
 

          }
 

-         

+ 

          if (pre_sdn && post_sdn && slapi_sdn_compare(pre_sdn, post_sdn) == 0) {
 

              /* Regarding memberof plugin, this rename is a no-op
 

               * but it can be expensive to process it. So skip it
 
@@ -1466,6 +1466,7 @@ 
 

  int
 

  memberof_modop_one_replace_r(Slapi_PBlock *pb, MemberOfConfig *config, int mod_op, Slapi_DN *group_sdn, Slapi_DN *op_this_sdn, Slapi_DN *replace_with_sdn, Slapi_DN *op_to_sdn, memberofstringll *stack)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      int rc = 0;
 

      LDAPMod mod;
 

      LDAPMod replace_mod;
 
@@ -1515,8 +1516,7 @@ 
 

      }
 

  

      /* determine if this is a group op or single entry */
 

-     slapi_search_internal_get_entry(op_to_sdn, config->groupattrs,
 

-                                     &e, memberof_get_plugin_id());
 

+     slapi_search_get_entry(&entry_pb, op_to_sdn, config->groupattrs, &e, memberof_get_plugin_id());
 

      if (!e) {
 

          /* In the case of a delete, we need to worry about the
 

           * missing entry being a nested group.  There's a small
 
@@ -1751,7 +1751,7 @@ 
 

  bail:
 

      slapi_value_free(&to_dn_val);
 

      slapi_value_free(&this_dn_val);
 

-     slapi_entry_free(e);
 

+     slapi_search_get_entry_done(&entry_pb);
 

      return rc;
 

  }
 

  
@@ -2368,6 +2368,7 @@ 
 

  int
 

  memberof_is_direct_member(MemberOfConfig *config, Slapi_Value *groupdn, Slapi_Value *memberdn)
 

  {
 

+     Slapi_PBlock *pb = NULL;
 

      int rc = 0;
 

      Slapi_DN *sdn = 0;
 

      Slapi_Entry *group_e = 0;
 
@@ -2376,8 +2377,8 @@ 
 

  

      sdn = slapi_sdn_new_normdn_byref(slapi_value_get_string(groupdn));
 

  

-     slapi_search_internal_get_entry(sdn, config->groupattrs,
 

-                                     &group_e, memberof_get_plugin_id());
 

+     slapi_search_get_entry(&pb, sdn, config->groupattrs,
 

+                            &group_e, memberof_get_plugin_id());
 

  

      if (group_e) {
 

          /* See if memberdn is referred to by any of the group attributes. */
 
@@ -2388,9 +2389,8 @@ 
 

                  break;
 

              }
 

          }
 

- 

-         slapi_entry_free(group_e);
 

      }
 

+     slapi_search_get_entry_done(&pb);
 

  

      slapi_sdn_free(&sdn);
 

      return rc;

file modified
+5 -5 
@@ -749,22 +749,22 @@ 
 

              if (pam_passthru_check_suffix(cfg, bind_sdn) == LDAP_SUCCESS) {
 

                  if (cfg->slapi_filter) {
 

                      /* A filter is configured, so see if the bind entry is a match. */
 

+                     Slapi_PBlock *entry_pb = NULL;
 

                      Slapi_Entry *test_e = NULL;
 

  

                      /* Fetch the bind entry */
 

-                     slapi_search_internal_get_entry(bind_sdn, NULL, &test_e,
 

-                                                     pam_passthruauth_get_plugin_identity());
 

+                     slapi_search_get_entry(&entry_pb, bind_sdn, NULL, &test_e,
 

+                                            pam_passthruauth_get_plugin_identity());
 

  

                      /* If the entry doesn't exist, just fall through to the main server code */
 

                      if (test_e) {
 

                          /* Evaluate the filter. */
 

                          if (LDAP_SUCCESS == slapi_filter_test_simple(test_e, cfg->slapi_filter)) {
 

                              /* This is a match. */
 

-                             slapi_entry_free(test_e);
 

+                             slapi_search_get_entry_done(&entry_pb);
 

                              goto done;
 

                          }
 

- 

-                         slapi_entry_free(test_e);
 

+                         slapi_search_get_entry_done(&entry_pb);
 

                      }
 

                  } else {
 

                      /* There is no filter to check, so this is a match. */

file modified
+4 -3 
@@ -81,11 +81,12 @@ 
 

  static char *
 

  derive_from_bind_entry(Slapi_PBlock *pb, const Slapi_DN *bindsdn, MyStrBuf *pam_id, char *map_ident_attr, int *locked)
 

  {
 

+ 	Slapi_PBlock *entry_pb = NULL;
 

      Slapi_Entry *entry = NULL;
 

      char *attrs[] = {NULL, NULL};
 

      attrs[0] = map_ident_attr;
 

-     int rc = slapi_search_internal_get_entry((Slapi_DN *)bindsdn, attrs, &entry,
 

-                                              pam_passthruauth_get_plugin_identity());
 

+     int32_t rc = slapi_search_get_entry(&entry_pb, (Slapi_DN *)bindsdn, attrs, &entry,
 

+                                         pam_passthruauth_get_plugin_identity());
 

  

      if (rc != LDAP_SUCCESS) {
 

          slapi_log_err(SLAPI_LOG_ERR, PAM_PASSTHRU_PLUGIN_SUBSYSTEM,
 
@@ -108,7 +109,7 @@ 
 

          init_my_str_buf(pam_id, val);
 

      }
 

  

-     slapi_entry_free(entry);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      return pam_id->str;
 

  }

file modified
+4 -5 
@@ -526,6 +526,7 @@ 
 

  static int
 

  pam_passthru_preop(Slapi_PBlock *pb, int modtype)
 

  {
 

+ 	Slapi_PBlock *entry_pb = NULL;
 

      Slapi_DN *sdn = NULL;
 

      Slapi_Entry *e = NULL;
 

      LDAPMod **mods;
 
@@ -555,8 +556,8 @@ 
 

          case LDAP_CHANGETYPE_MODIFY:
 

              /* Fetch the entry being modified so we can
 

               * create the resulting entry for validation. */
 

-             slapi_search_internal_get_entry(sdn, 0, &e,
 

-                                             pam_passthruauth_get_plugin_identity());
 

+             slapi_search_get_entry(&entry_pb, sdn, 0, &e,
 

+                                    pam_passthruauth_get_plugin_identity());
 

  

              /* If the entry doesn't exist, just bail and
 

               * let the server handle it. */
 
@@ -576,9 +577,6 @@ 
 

                      /* Don't bail here, as we need to free the entry. */
 

                  }
 

              }
 

- 

-             /* Free the entry. */
 

-             slapi_entry_free(e);
 

              break;
 

          case LDAP_CHANGETYPE_DELETE:
 

          case LDAP_CHANGETYPE_MODDN:
 
@@ -591,6 +589,7 @@ 
 

      }
 

  

  bail:
 

+     slapi_search_get_entry_done(&entry_pb);
 

      /* If we are refusing the operation, return the result to the client. */
 

      if (ret) {
 

          slapi_send_ldap_result(pb, ret, NULL, returntext, 0, NULL);

file modified
+3 -2 
@@ -469,7 +469,8 @@ 
 

           */
 

          /* Get suffix */
 

          Slapi_Entry *suffix = NULL;
 

-         rc = slapi_search_internal_get_entry(area_sdn, NULL, &suffix, repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION));
 

+         Slapi_PBlock *suffix_pb = NULL;
 

+         rc = slapi_search_get_entry(&suffix_pb, area_sdn, NULL, &suffix, repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION));
 

          if (rc) {
 

              slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "repl5_tot_run -  Unable to "
 

                                                             "get the suffix entry \"%s\".\n",
 
@@ -517,7 +518,7 @@ 
 

                                       LDAP_SCOPE_SUBTREE, "(parentid>=1)", NULL, 0, ctrls, NULL,
 

                                       repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION), OP_FLAG_BULK_IMPORT);
 

          cb_data.num_entries = 0UL;
 

-         slapi_entry_free(suffix);
 

+         slapi_search_get_entry_done(&suffix_pb);
 

      } else {
 

          /* Original total update */
 

          /* we need to provide managedsait control so that referral entries can

file modified
+12 -11 
@@ -1254,6 +1254,7 @@ 
 

  static int
 

  preop_modrdn(Slapi_PBlock *pb)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      int result = LDAP_SUCCESS;
 

      Slapi_Entry *e = NULL;
 

      Slapi_Value *sv_requiredObjectClass = NULL;
 
@@ -1351,7 +1352,7 @@ 
 

  

      /* Get the entry that is being renamed so we can make a dummy copy
 

       * of what it will look like after the rename. */
 

-     err = slapi_search_internal_get_entry(sdn, NULL, &e, plugin_identity);
 

+     err = slapi_search_get_entry(&entry_pb, sdn, NULL, &e, plugin_identity);
 

      if (err != LDAP_SUCCESS) {
 

          result = uid_op_error(35);
 

          /* We want to return a no such object error if the target doesn't exist. */
 
@@ -1371,24 +1372,24 @@ 
 

  

  

      /*
 

-          * Check if it has the required object class
 

-          */
 

+      * Check if it has the required object class
 

+      */
 

      if (requiredObjectClass &&
 

          !slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS, sv_requiredObjectClass)) {
 

          break;
 

      }
 

  

      /*
 

-          * Find any unique attribute data in the new RDN
 

-          */
 

+      * Find any unique attribute data in the new RDN
 

+      */
 

      for (i = 0; attrNames && attrNames[i]; i++) {
 

          err = slapi_entry_attr_find(e, attrNames[i], &attr);
 

          if (!err) {
 

              /*
 

-                  * Passed all the requirements - this is an operation we
 

-                  * need to enforce uniqueness on. Now find all parent entries
 

-                  * with the marker object class, and do a search for each one.
 

-                  */
 

+              * Passed all the requirements - this is an operation we
 

+              * need to enforce uniqueness on. Now find all parent entries
 

+              * with the marker object class, and do a search for each one.
 

+              */
 

              if (NULL != markerObjectClass) {
 

                  /* Subtree defined by location of marker object class */
 

                  result = findSubtreeAndSearch(slapi_entry_get_sdn(e), attrNames, attr, NULL,
 
@@ -1407,8 +1408,8 @@ 
 

      END
 

          /* Clean-up */
 

          slapi_value_free(&sv_requiredObjectClass);
 

-     if (e)
 

-         slapi_entry_free(e);
 

+ 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      if (result) {
 

          slapi_log_err(SLAPI_LOG_PLUGIN, plugin_name,

file modified
+3 -8 
@@ -1916,18 +1916,13 @@ 
 

              char *root_dn = config_get_ldapi_root_dn();
 

  

              if (root_dn) {
 

+                 Slapi_PBlock *entry_pb = NULL;
 

                  Slapi_DN *edn = slapi_sdn_new_dn_byref(
 

                      slapi_dn_normalize(root_dn));
 

                  Slapi_Entry *e = 0;
 

  

                  /* root might be locked too! :) */
 

-                 ret = slapi_search_internal_get_entry(
 

-                     edn, 0,
 

-                     &e,
 

-                     (void *)plugin_get_default_component_id()
 

- 

-                         );
 

- 

+                 ret = slapi_search_get_entry(&entry_pb, edn, 0, &e, (void *)plugin_get_default_component_id());
 

                  if (0 == ret && e) {
 

                      ret = slapi_check_account_lock(
 

                          0, /* pb not req */
 
@@ -1955,7 +1950,7 @@ 
 

              root_map_free:
 

                  /* root_dn consumed by bind creds set */
 

                  slapi_sdn_free(&edn);
 

-                 slapi_entry_free(e);
 

+                 slapi_search_get_entry_done(&entry_pb);
 

                  ret = 0;
 

              }
 

          }

file modified
+8 -4 
@@ -592,6 +592,7 @@ 
 

  static void
 

  op_shared_modify(Slapi_PBlock *pb, int pw_change, char *old_pw)
 

  {
 

+     Slapi_PBlock *entry_pb = NULL;
 

      Slapi_Backend *be = NULL;
 

      Slapi_Entry *pse;
 

      Slapi_Entry *referral;
 
@@ -723,7 +724,7 @@ 
 

       * 2. If yes, then if the mods contain any passwdpolicy specific attributes.
 

       * 3. If yes, then it invokes corrosponding checking function.
 

       */
 

-     if (!repl_op && !internal_op && normdn && (e = get_entry(pb, normdn))) {
 

+     if (!repl_op && !internal_op && normdn && slapi_search_get_entry(&entry_pb, sdn, NULL, &e, NULL) == LDAP_SUCCESS) {
 

          Slapi_Value target;
 

          slapi_value_init(&target);
 

          slapi_value_set_string(&target, "passwordpolicy");
 
@@ -1072,7 +1073,7 @@ 
 

      slapi_entry_free(epre);
 

      slapi_entry_free(epost);
 

  }
 

-     slapi_entry_free(e);
 

+     slapi_search_get_entry_done(&entry_pb);
 

  

      if (be)
 

          slapi_be_Unlock(be);
 
@@ -1202,12 +1203,13 @@ 
 

      if (!internal_op) {
 

          /* slapi_acl_check_mods needs an array of LDAPMods, but
 

           * we're really only interested in the one password mod. */
 

+         Slapi_PBlock *entry_pb = NULL;
 

          LDAPMod *mods[2];
 

          mods[0] = mod;
 

          mods[1] = NULL;
 

  

          /* We need to actually fetch the target here to use for ACI checking. */
 

-         slapi_search_internal_get_entry(&sdn, NULL, &e, (void *)plugin_get_default_component_id());
 

+         slapi_search_get_entry(&entry_pb, &sdn, NULL, &e, NULL);
 

  

          /* Create a bogus entry with just the target dn if we were unable to
 

           * find the actual entry.  This will only be used for checking the ACIs. */
 
@@ -1238,9 +1240,12 @@ 
 

              }
 

              send_ldap_result(pb, res, NULL, errtxt, 0, NULL);
 

              slapi_ch_free_string(&errtxt);
 

+             slapi_search_get_entry_done(&entry_pb);
 

              rc = -1;
 

              goto done;
 

          }
 

+         /* done with slapi entry e */
 

+         slapi_search_get_entry_done(&entry_pb);
 

  

          /*
 

           * If this mod is being performed by a password administrator/rootDN,
 
@@ -1353,7 +1358,6 @@ 
 

      valuearray_free(&values);
 

  

  done:
 

-     slapi_entry_free(e);
 

      slapi_sdn_done(&sdn);
 

      slapi_ch_free_string(&proxydn);
 

      slapi_ch_free_string(&proxystr);

file modified
+48 
@@ -882,3 +882,51 @@ 
 

      int_search_pb = NULL;
 

      return rc;
 

  }
 

+ 

+ int32_t
 

+ slapi_search_get_entry(Slapi_PBlock **pb, Slapi_DN *dn, char **attrs, Slapi_Entry **ret_entry, void *component_identity)
 

+ {
 

+     Slapi_Entry **entries = NULL;
 

+     int32_t rc = 0;
 

+     void *component = component_identity;
 

+ 

+     if (ret_entry) {
 

+         *ret_entry = NULL;
 

+     }
 

+ 

+     if (component == NULL) {
 

+         component = (void *)plugin_get_default_component_id();
 

+     }
 

+ 

+     if (*pb == NULL) {
 

+         *pb = slapi_pblock_new();
 

+     }
 

+     slapi_search_internal_set_pb(*pb, slapi_sdn_get_dn(dn), LDAP_SCOPE_BASE,
 

+         "(|(objectclass=*)(objectclass=ldapsubentry))",
 

+         attrs, 0, NULL, NULL, component, 0 );
 

+     slapi_search_internal_pb(*pb);
 

+     slapi_pblock_get(*pb, SLAPI_PLUGIN_INTOP_RESULT, &rc);
 

+     if (LDAP_SUCCESS == rc) {
 

+         slapi_pblock_get(*pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, &entries);
 

+         if (NULL != entries && NULL != entries[0]) {
 

+             /* Only need to dup the entry if the caller passed ret_entry in. */
 

+             if (ret_entry) {
 

+                 *ret_entry = entries[0];
 

+             }
 

+         } else {
 

+             rc = LDAP_NO_SUCH_OBJECT;
 

+         }
 

+     }
 

+ 

+     return rc;
 

+ }
 

+ 

+ void
 

+ slapi_search_get_entry_done(Slapi_PBlock **pb)
 

+ {
 

+     if (pb && *pb) {
 

+         slapi_free_search_results_internal(*pb);
 

+         slapi_pblock_destroy(*pb);
 

+         *pb = NULL;
 

+     }
 

+ }

file modified
+4 -9 
@@ -305,22 +305,17 @@ 
 

  int
 

  reslimit_update_from_dn(Slapi_Connection *conn, Slapi_DN *dn)
 

  {
 

-     Slapi_Entry *e;
 

+     Slapi_PBlock *pb = NULL;
 

+     Slapi_Entry *e = NULL;
 

      int rc;
 

  

-     e = NULL;
 

      if (dn != NULL) {
 

- 

          char **attrs = reslimit_get_registered_attributes();
 

-         (void)slapi_search_internal_get_entry(dn, attrs, &e, reslimit_componentid);
 

+         slapi_search_get_entry(&pb, dn, attrs, &e, reslimit_componentid);
 

          charray_free(attrs);
 

      }
 

- 

      rc = reslimit_update_from_entry(conn, e);
 

- 

-     if (NULL != e) {
 

-         slapi_entry_free(e);
 

-     }
 

+     slapi_search_get_entry_done(&pb);
 

  

      return (rc);
 

  }

file modified
+3 -4 
@@ -341,6 +341,7 @@ 
 

  static void
 

  schema_load_repl_policy(const char *dn, repl_schema_policy_t *replica)
 

  {
 

+     Slapi_PBlock *pb = NULL;
 

      Slapi_DN sdn;
 

      Slapi_Entry *entry = NULL;
 

      schema_item_t *schema_item, *next;
 
@@ -369,8 +370,7 @@ 
 

  

      /* Load the replication policy of the schema  */
 

      slapi_sdn_init_dn_byref(&sdn, dn);
 

-     if (slapi_search_internal_get_entry(&sdn, NULL, &entry, plugin_get_default_component_id()) == LDAP_SUCCESS) {
 

- 

+     if (slapi_search_get_entry(&pb, &sdn, NULL, &entry, plugin_get_default_component_id()) == LDAP_SUCCESS) {
 

          /* fill the policies (accept/reject) regarding objectclass */
 

          schema_policy_add_action(entry, ATTR_SCHEMA_UPDATE_OBJECTCLASS_ACCEPT, &replica->objectclasses);
 

          schema_policy_add_action(entry, ATTR_SCHEMA_UPDATE_OBJECTCLASS_REJECT, &replica->objectclasses);
 
@@ -378,9 +378,8 @@ 
 

          /* fill the policies (accept/reject) regarding attribute */
 

          schema_policy_add_action(entry, ATTR_SCHEMA_UPDATE_ATTRIBUTE_ACCEPT, &replica->attributes);
 

          schema_policy_add_action(entry, ATTR_SCHEMA_UPDATE_ATTRIBUTE_REJECT, &replica->attributes);
 

- 

-         slapi_entry_free(entry);
 

      }
 

+     slapi_search_get_entry_done(&pb);
 

      slapi_sdn_done(&sdn);
 

  }

file modified
+22 -1 
@@ -5972,7 +5972,7 @@ 
 

  

  /*
 

   * slapi_search_internal_get_entry() finds an entry given a dn.  It returns
 

-  * an LDAP error code (LDAP_SUCCESS if all goes well).
 

+  * an LDAP error code (LDAP_SUCCESS if all goes well).  Caller must free ret_entry
 

   */
 

  int slapi_search_internal_get_entry(Slapi_DN *dn, char **attrlist, Slapi_Entry **ret_entry, void *caller_identity);
 

  
@@ -8304,6 +8304,27 @@ 
 

  /* helper function */
 

  const char * slapi_fetch_attr(Slapi_Entry *e, const char *attrname, char *default_val);
 

  

+ /**
 

+  * Get a Slapi_Entry via an internal search.  The caller then needs to call
 

+  * slapi_get_entry_done() to free any resources allocated to get the entry
 

+  *
 

+  * \param pb - slapi_pblock pointer (the function will allocate if necessary)
 

+  * \param dn - Slapi_DN of the entry to retrieve
 

+  * \param attrs - char list of attributes to get
 

+  * \param ret_entry - pointer to a Slapi_entry wer the returned entry is stored
 

+  * \param component_identity - plugin component
 

+  *
 

+  * \return - ldap result code
 

+  */
 

+ int32_t slapi_search_get_entry(Slapi_PBlock **pb, Slapi_DN *dn, char **attrs, Slapi_Entry **ret_entry, void *component_identity);
 

+ 

+ /**
 

+  * Free the resources allocated by slapi_search_get_entry()
 

+  *
 

+  * \param pb - slapi_pblock pointer
 

+  */
 

+ void slapi_search_get_entry_done(Slapi_PBlock **pb);
 

+ 

  #ifdef __cplusplus
 

  }
 

  #endif

Description:

So the problem is that slapi_search_internal_get_entry() duplicates the entry twice. It does that as a convenience where it will allocate a pblock, do the search, copy the entry, free search results from the pblock, and then free the pblock itself. I basically split this function into two functions. One function allocates the pblock, does the search and returns the entry. The other function frees the entries and pblock.

99% of time when we call slapi_search_internal_get_entry() we are just reading it and freeing it. It's not being consumed. In these cases we can use the two function approach which eliminates an extra slapi_entry_dup(). Over the life time of an operation/connection we can save quite a bit of mallocing/freeing. This could also help with memory fragmentation.

ASAN: passed

relates: https://pagure.io/389-ds-base/issue/51076

Metadata Update from @mreynolds:
- Request assigned
3 years ago

Combined with the previous patch here are some numbers doing a search, mods, and deletes with and without these patches:

Search (bind as user)
=========================
No Patch:  6 dups
Patch:     4 dups


Modify (change generic attribute)
=========================
No Patch:  8 dups
Patch:     5 dups


Modify (MO plugin - add member to group)
=========================
No Patch:  16 dups
Patch:     10 dups

     *  Biggest change of all the ops I tested


Delete
=========================
No patch:  5 dups
Patch:     3 dups


Delete (RI plugin - delete a member of a group)
=========================
No Patch:  9 dups
Patch:     8 dups


Delete (MO plugin - delete group)
=========================
No Patch:  10 dups
Patch:      8 dups

This all looks reasonable to me, but I think I'd like @tbordaz to check too. I assume this has passed tests?

Nice performance boost. It looks it gives a 10%-15% improvement on the search path (excluding auth impact and with same #workers). From the tests, I have a doubt about disabling virtual attribute, IMHO it is not realistic for most of the deployment because some users may ignore what is a virtual attribute and if it is safe to disable them.

Here are the perf tests:
(4 worker threads, virtual attributes are off, MEP is off, db env is on /dev/shm)

Not to see be too annoying, but I think too many changes were made in this test. In this case I'd like to see just my patch in place. Since I cleaned up the extra dupping in MEP I'd like to leave that plugin on. So it would be great to see:

  • Just my patch
  • Just /dev/shm set (no patch)
  • Just vattr off (no patch) - I wonder since Thierry changed the locking if this is still a factor.

Sorry for the delay. I had to rerun tests several times, because changing db home dir during the tests screwed up my ds instance several times.
Anyway, here are the requested results:
https://fedorapeople.org/groups/389ds/ci/pr51085_v2/search.html
https://fedorapeople.org/groups/389ds/ci/pr51085_v2/modify.html

And the same with 4 worker threads:
https://fedorapeople.org/groups/389ds/ci/pr51085_v2/search%20(tn=4).html
https://fedorapeople.org/groups/389ds/ci/pr51085_v2/modify%20(tn=4).html

Doesn't seem to really have any significant impact on performance, but it's still a lot less dupping/freeing for every operation.

Turning off vattrs still seems to have the biggest impact.

Right, those perf measurements open more questions than it answer ! Anyway it is very precious results. Thanks

Regarding vattr impact, do the searches ask for all attributes or a specific set ?. If requesting only dn, it should be similar to vattr-off.

In search tests givenName, sn and mail are requested.

I still think we should push this commit, can someone please review it and give it an official Ack (or nack)?

to be honest the fix in mep.c is complex to be evaluated.
entry_pb and 'e' are used in several places. In addition apply_mods are done on the entry 'e' that looks unexpected as your change is for read-only entries.
Also 'e' can be saved in config_copy->template_entry, it is not clear if this reference will be used after you clear the pb_results.

In short, this specific part needs more time to be reviewed. Could it be moved out from your patch ?

The rest of the patch looks okay to me. Need a second looks at mep.c

rebased onto 68ab6a8
3 years ago

to be honest the fix in mep.c is complex to be evaluated.
entry_pb and 'e' are used in several places. In addition apply_mods are done on the entry 'e' that looks unexpected as your change is for read-only entries.
Also 'e' can be saved in config_copy->template_entry, it is not clear if this reference will be used after you clear the pb_results.
In short, this specific part needs more time to be reviewed. Could it be moved out from your patch ?

You're right there was one area where it was not safe where it get consumed into the config. I could just dup the entry in that case, but I just removed it all. Please review...

I already reviewed the previous patch and had a concern only for mep.c
As you postpone the change of mep.c, the rest of the patch looks good to me. ACK

Pull-Request has been merged by mreynolds
3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4138

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
No Tags
Changes Summary 17
+3 -3
file changed
ldap/servers/plugins/acctpolicy/acct_config.c
+16 -20
file changed
ldap/servers/plugins/acctpolicy/acct_plugin.c
+3 -3
file changed
ldap/servers/plugins/acctpolicy/acct_util.c
+6 -11
file changed
ldap/servers/plugins/automember/automember.c
+9 -14
file changed
ldap/servers/plugins/dna/dna.c
+8 -8
file changed
ldap/servers/plugins/memberof/memberof.c
+5 -5
file changed
ldap/servers/plugins/pam_passthru/pam_ptconfig.c
+4 -3
file changed
ldap/servers/plugins/pam_passthru/pam_ptimpl.c
+4 -5
file changed
ldap/servers/plugins/pam_passthru/pam_ptpreop.c
+3 -2
file changed
ldap/servers/plugins/replication/repl5_tot_protocol.c
+12 -11
file changed
ldap/servers/plugins/uiduniq/uid.c
+3 -8
file changed
ldap/servers/slapd/daemon.c
+8 -4
file changed
ldap/servers/slapd/modify.c
+48 -0
file changed
ldap/servers/slapd/plugin_internal_op.c
+4 -9
file changed
ldap/servers/slapd/resourcelimit.c
+3 -4
file changed
ldap/servers/slapd/schema.c
+22 -1
file changed
ldap/servers/slapd/slapi-plugin.h
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.