389-commits May 2014

389-commits@lists.fedoraproject.org

4 participants
64 discussions

Branch '389-ds-base-1.3.1' - ldap/servers
by Noriko Hosoi 16 May '14

16 May '14

ldap/servers/slapd/back-ldbm/dbhelp.c | 6 ++ ldap/servers/slapd/back-ldbm/dblayer.c | 60 ++++++++++++------------- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 4 - 3 files changed, 39 insertions(+), 31 deletions(-) New commits: commit 04ecdb08d45795646becb3535c477c006f5f7c6b Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Fri May 16 10:05:43 2014 -0700 Ticket #47804 - db2bak.pl error with changelogdb Bug description: Backup utility db2bak[.pl] copies not just backend db files but also changelog db files, which is not associated with the backend instance, but the backend code blindly expected it. Fix description: If the copying directory is a changelog db dir, skip retrieving the backend instance info and just copy the files underneath https://fedorahosted.org/389/ticket/47804 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) (cherry picked from commit a6c24c5bf1216e6dceaf014a25ce60a5a714635c) diff --git a/ldap/servers/slapd/back-ldbm/dbhelp.c b/ldap/servers/slapd/back-ldbm/dbhelp.c index b0d17d3..797e2ae 100644 --- a/ldap/servers/slapd/back-ldbm/dbhelp.c +++ b/ldap/servers/slapd/back-ldbm/dbhelp.c @@ -139,6 +139,12 @@ dblayer_copy_file_keybykey(DB_ENV *env, (*(p + sizeof(LDBM_ENTRYRDN_STR) - 1) == '.')) { /* entryrdn.db */ struct attrinfo *ai = NULL; + if (NULL == inst) { + LDAPDebug0Args(LDAP_DEBUG_ANY, + "dblayer_copy_file_keybykey(entryrdn), " + "dup_cmp_fn cannot be retrieved since inst is NULL.\n"); + goto error; + } ainfo_get(inst->inst_be, LDBM_ENTRYRDN_STR, &ai); if (ai->ai_dup_cmp_fn) { /* If set, use the special dup compare callback */ diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c index 844b3b7..89effd4 100644 --- a/ldap/servers/slapd/back-ldbm/dblayer.c +++ b/ldap/servers/slapd/back-ldbm/dblayer.c @@ -5633,17 +5633,15 @@ dblayer_copy_directory(struct ldbminfo *li, char *dest_dir, int restore, int *cnt, - int instance_dir_flag, int indexonly, - int resetlsns) + int resetlsns, + int is_changelog) { dblayer_private *priv = NULL; char *new_src_dir = NULL; char *new_dest_dir = NULL; PRDir *dirhandle = NULL; PRDirEntry *direntry = NULL; - size_t filename_length = 0; - size_t offset = 0; char *compare_piece = NULL; char *filename1; char *filename2; @@ -5652,8 +5650,8 @@ dblayer_copy_directory(struct ldbminfo *li, char *inst_dirp = NULL; char inst_dir[MAXPATHLEN]; char sep; - int suffix_len = 0; - ldbm_instance *inst = NULL; + int src_is_fullpath = 0; + ldbm_instance *inst = NULL; if (!src_dir || '\0' == *src_dir) { @@ -5677,20 +5675,28 @@ dblayer_copy_directory(struct ldbminfo *li, else relative_instance_name++; - inst = ldbm_instance_find_by_name(li, relative_instance_name); - if (NULL == inst) { - LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " - "Instance path %s could be invalid.\n", - relative_instance_name, src_dir, 0); - return return_value; + if (is_fullpath(src_dir)) { + src_is_fullpath = 1; + } + if (is_changelog) { + if (!src_is_fullpath) { + LDAPDebug1Arg(LDAP_DEBUG_ANY, "Changelogdir \"%s\" is not full path; " + "Skipping it.\n", src_dir); + return 0; + } + } else { + inst = ldbm_instance_find_by_name(li, relative_instance_name); + if (NULL == inst) { + LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " + "Instance path %s could be invalid.\n", + relative_instance_name, src_dir, 0); + return return_value; + } } - if (is_fullpath(src_dir)) - { + if (src_is_fullpath) { new_src_dir = src_dir; - } - else - { + } else { int len; inst_dirp = dblayer_get_full_inst_dir(inst->inst_li, inst, @@ -5717,7 +5723,6 @@ dblayer_copy_directory(struct ldbminfo *li, return return_value; } - suffix_len = sizeof(LDBM_SUFFIX) - 1; while (NULL != (direntry = PR_ReadDir(dirhandle, PR_SKIP_DOT | PR_SKIP_DOT_DOT))) { @@ -5731,15 +5736,10 @@ dblayer_copy_directory(struct ldbminfo *li, continue; } - /* Look at the last three characters in the filename */ - filename_length = strlen(direntry->name); - if (filename_length > suffix_len) { - offset = filename_length - suffix_len; - } else { - offset = 0; + compare_piece = PL_strrchr((char *)direntry->name, '.'); + if (NULL == compare_piece) { + compare_piece = (char *)direntry->name; } - compare_piece = (char *)direntry->name + offset; - /* rename .db3 -> .db4 or .db4 -> .db */ if (0 == strcmp(compare_piece, LDBM_FILENAME_SUFFIX) || 0 == strcmp(compare_piece, LDBM_SUFFIX_OLD) || @@ -5839,6 +5839,7 @@ out: /* * Get changelogdir from cn=changelog5,cn=config * The value does not have trailing spaces nor slashes. + * The changelogdir value must be a fullpath. */ static int _dblayer_get_changelogdir(struct ldbminfo *li, char **changelogdir) @@ -6054,7 +6055,7 @@ dblayer_backup(struct ldbminfo *li, char *dest_dir, Slapi_Task *task) return_value = dblayer_copy_directory(li, task, changelogdir, changelog_destdir, 0 /* backup */, - &cnt, 0, 0, 0); + &cnt, 0, 0, 1); if (return_value) { LDAPDebug(LDAP_DEBUG_ANY, "Backup: error in copying directory " @@ -6726,7 +6727,8 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * /* Get the parent dir of changelogdir */ *cldirname = '\0'; return_value = dblayer_copy_directory(li, task, filename1, - changelogdir, 1 /* restore */, &cnt, 0, 0, 0); + changelogdir, 1 /* restore */, + &cnt, 0, 0, 1); *cldirname = '/'; if (return_value) { LDAPDebug1Arg(LDAP_DEBUG_ANY, @@ -6758,7 +6760,7 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * restore_dir = inst->inst_parent_dir_name; /* If we're doing a partial restore, we need to reset the LSNs on the data files */ if (dblayer_copy_directory(li, task, filename1, - restore_dir, 1 /* restore */, &cnt, 0, 0, (bename) ? 1 : 0) == 0) + restore_dir, 1 /* restore */, &cnt, 0, (bename) ? 1 : 0, 0) == 0) continue; else { diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 2f35702..732568d 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -143,8 +143,8 @@ int dblayer_backup(struct ldbminfo *li, char *destination_directory, int dblayer_restore(struct ldbminfo *li, char* source_directory, Slapi_Task *task, char *bename); int dblayer_copy_directory(struct ldbminfo *li, Slapi_Task *task, char *instance_dir, char *destination_dir, - int restore, int *cnt, int instance_dir_flag, - int indexonly, int resetlsns); + int restore, int *cnt, int indexonly, + int resetlsns, int is_changelog); int dblayer_copyfile(char* source, char * destination, int overwrite, int mode); int dblayer_delete_instance_dir(backend *be); int dblayer_delete_database(struct ldbminfo *li);

1 0

Branch '389-ds-base-1.2.11' - ldap/servers
by Noriko Hosoi 16 May '14

16 May '14

ldap/servers/slapd/back-ldbm/dbhelp.c | 6 ++ ldap/servers/slapd/back-ldbm/dblayer.c | 60 ++++++++++++------------- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 4 - 3 files changed, 39 insertions(+), 31 deletions(-) New commits: commit a6c24c5bf1216e6dceaf014a25ce60a5a714635c Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Fri May 16 10:05:43 2014 -0700 Ticket #47804 - db2bak.pl error with changelogdb Bug description: Backup utility db2bak[.pl] copies not just backend db files but also changelog db files, which is not associated with the backend instance, but the backend code blindly expected it. Fix description: If the copying directory is a changelog db dir, skip retrieving the backend instance info and just copy the files underneath https://fedorahosted.org/389/ticket/47804 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) diff --git a/ldap/servers/slapd/back-ldbm/dbhelp.c b/ldap/servers/slapd/back-ldbm/dbhelp.c index b0d17d3..797e2ae 100644 --- a/ldap/servers/slapd/back-ldbm/dbhelp.c +++ b/ldap/servers/slapd/back-ldbm/dbhelp.c @@ -139,6 +139,12 @@ dblayer_copy_file_keybykey(DB_ENV *env, (*(p + sizeof(LDBM_ENTRYRDN_STR) - 1) == '.')) { /* entryrdn.db */ struct attrinfo *ai = NULL; + if (NULL == inst) { + LDAPDebug0Args(LDAP_DEBUG_ANY, + "dblayer_copy_file_keybykey(entryrdn), " + "dup_cmp_fn cannot be retrieved since inst is NULL.\n"); + goto error; + } ainfo_get(inst->inst_be, LDBM_ENTRYRDN_STR, &ai); if (ai->ai_dup_cmp_fn) { /* If set, use the special dup compare callback */ diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c index a26c2f0..fe7a617 100644 --- a/ldap/servers/slapd/back-ldbm/dblayer.c +++ b/ldap/servers/slapd/back-ldbm/dblayer.c @@ -5526,17 +5526,15 @@ dblayer_copy_directory(struct ldbminfo *li, char *dest_dir, int restore, int *cnt, - int instance_dir_flag, int indexonly, - int resetlsns) + int resetlsns, + int is_changelog) { dblayer_private *priv = NULL; char *new_src_dir = NULL; char *new_dest_dir = NULL; PRDir *dirhandle = NULL; PRDirEntry *direntry = NULL; - size_t filename_length = 0; - size_t offset = 0; char *compare_piece = NULL; char *filename1; char *filename2; @@ -5545,8 +5543,8 @@ dblayer_copy_directory(struct ldbminfo *li, char *inst_dirp = NULL; char inst_dir[MAXPATHLEN]; char sep; - int suffix_len = 0; - ldbm_instance *inst = NULL; + int src_is_fullpath = 0; + ldbm_instance *inst = NULL; if (!src_dir || '\0' == *src_dir) { @@ -5570,20 +5568,28 @@ dblayer_copy_directory(struct ldbminfo *li, else relative_instance_name++; - inst = ldbm_instance_find_by_name(li, relative_instance_name); - if (NULL == inst) { - LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " - "Instance path %s could be invalid.\n", - relative_instance_name, src_dir, 0); - return return_value; + if (is_fullpath(src_dir)) { + src_is_fullpath = 1; + } + if (is_changelog) { + if (!src_is_fullpath) { + LDAPDebug1Arg(LDAP_DEBUG_ANY, "Changelogdir \"%s\" is not full path; " + "Skipping it.\n", src_dir); + return 0; + } + } else { + inst = ldbm_instance_find_by_name(li, relative_instance_name); + if (NULL == inst) { + LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " + "Instance path %s could be invalid.\n", + relative_instance_name, src_dir, 0); + return return_value; + } } - if (is_fullpath(src_dir)) - { + if (src_is_fullpath) { new_src_dir = src_dir; - } - else - { + } else { int len; inst_dirp = dblayer_get_full_inst_dir(inst->inst_li, inst, @@ -5610,7 +5616,6 @@ dblayer_copy_directory(struct ldbminfo *li, return return_value; } - suffix_len = sizeof(LDBM_SUFFIX) - 1; while (NULL != (direntry = PR_ReadDir(dirhandle, PR_SKIP_DOT | PR_SKIP_DOT_DOT))) { @@ -5624,15 +5629,10 @@ dblayer_copy_directory(struct ldbminfo *li, continue; } - /* Look at the last three characters in the filename */ - filename_length = strlen(direntry->name); - if (filename_length > suffix_len) { - offset = filename_length - suffix_len; - } else { - offset = 0; + compare_piece = PL_strrchr((char *)direntry->name, '.'); + if (NULL == compare_piece) { + compare_piece = (char *)direntry->name; } - compare_piece = (char *)direntry->name + offset; - /* rename .db3 -> .db4 or .db4 -> .db */ if (0 == strcmp(compare_piece, LDBM_FILENAME_SUFFIX) || 0 == strcmp(compare_piece, LDBM_SUFFIX_OLD) || @@ -5732,6 +5732,7 @@ out: /* * Get changelogdir from cn=changelog5,cn=config * The value does not have trailing spaces nor slashes. + * The changelogdir value must be a fullpath. */ static int _dblayer_get_changelogdir(struct ldbminfo *li, char **changelogdir) @@ -5947,7 +5948,7 @@ dblayer_backup(struct ldbminfo *li, char *dest_dir, Slapi_Task *task) return_value = dblayer_copy_directory(li, task, changelogdir, changelog_destdir, 0 /* backup */, - &cnt, 0, 0, 0); + &cnt, 0, 0, 1); if (return_value) { LDAPDebug(LDAP_DEBUG_ANY, "Backup: error in copying directory " @@ -6619,7 +6620,8 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * /* Get the parent dir of changelogdir */ *cldirname = '\0'; return_value = dblayer_copy_directory(li, task, filename1, - changelogdir, 1 /* restore */, &cnt, 0, 0, 0); + changelogdir, 1 /* restore */, + &cnt, 0, 0, 1); *cldirname = '/'; if (return_value) { LDAPDebug1Arg(LDAP_DEBUG_ANY, @@ -6651,7 +6653,7 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * restore_dir = inst->inst_parent_dir_name; /* If we're doing a partial restore, we need to reset the LSNs on the data files */ if (dblayer_copy_directory(li, task, filename1, - restore_dir, 1 /* restore */, &cnt, 0, 0, (bename) ? 1 : 0) == 0) + restore_dir, 1 /* restore */, &cnt, 0, (bename) ? 1 : 0, 0) == 0) continue; else { diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 499c92b..65cafea 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -137,8 +137,8 @@ int dblayer_backup(struct ldbminfo *li, char *destination_directory, int dblayer_restore(struct ldbminfo *li, char* source_directory, Slapi_Task *task, char *bename); int dblayer_copy_directory(struct ldbminfo *li, Slapi_Task *task, char *instance_dir, char *destination_dir, - int restore, int *cnt, int instance_dir_flag, - int indexonly, int resetlsns); + int restore, int *cnt, int indexonly, + int resetlsns, int is_changelog); int dblayer_copyfile(char* source, char * destination, int overwrite, int mode); int dblayer_delete_instance_dir(backend *be); int dblayer_delete_database(struct ldbminfo *li);

1 0

Branch '389-ds-base-1.3.2' - ldap/servers
by Noriko Hosoi 16 May '14

16 May '14

ldap/servers/slapd/back-ldbm/dbhelp.c | 6 ++ ldap/servers/slapd/back-ldbm/dblayer.c | 60 ++++++++++++------------- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 4 - 3 files changed, 39 insertions(+), 31 deletions(-) New commits: commit 072b9fe660c35cf71c9df678c96bf39e61b47687 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Thu May 15 18:57:21 2014 -0700 Ticket #47804 - db2bak.pl error with changelogdb Bug description: Backup utility db2bak[.pl] copies not just backend db files but also changelog db files, which is not associated with the backend instance, but the backup code blindly expected it. Fix description: If the copying directory is a changelog db dir, skip retrieving the backend instance info and just copy the files underneath. https://fedorahosted.org/389/ticket/47804 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) Signed-off-by: Noriko Hosoi <nhosoi(a)redhat.com> diff --git a/ldap/servers/slapd/back-ldbm/dbhelp.c b/ldap/servers/slapd/back-ldbm/dbhelp.c index 29c0247..40966ac 100644 --- a/ldap/servers/slapd/back-ldbm/dbhelp.c +++ b/ldap/servers/slapd/back-ldbm/dbhelp.c @@ -139,6 +139,12 @@ dblayer_copy_file_keybykey(DB_ENV *env, (*(p + sizeof(LDBM_ENTRYRDN_STR) - 1) == '.')) { /* entryrdn.db */ struct attrinfo *ai = NULL; + if (NULL == inst) { + LDAPDebug0Args(LDAP_DEBUG_ANY, + "dblayer_copy_file_keybykey(entryrdn), " + "dup_cmp_fn cannot be retrieved since inst is NULL.\n"); + goto error; + } ainfo_get(inst->inst_be, LDBM_ENTRYRDN_STR, &ai); if (ai->ai_dup_cmp_fn) { /* If set, use the special dup compare callback */ diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c index 6582c7a..6f3d258 100644 --- a/ldap/servers/slapd/back-ldbm/dblayer.c +++ b/ldap/servers/slapd/back-ldbm/dblayer.c @@ -5918,17 +5918,15 @@ dblayer_copy_directory(struct ldbminfo *li, char *dest_dir, int restore, int *cnt, - int instance_dir_flag, int indexonly, - int resetlsns) + int resetlsns, + int is_changelog) { dblayer_private *priv = NULL; char *new_src_dir = NULL; char *new_dest_dir = NULL; PRDir *dirhandle = NULL; PRDirEntry *direntry = NULL; - size_t filename_length = 0; - size_t offset = 0; char *compare_piece = NULL; char *filename1; char *filename2; @@ -5937,8 +5935,8 @@ dblayer_copy_directory(struct ldbminfo *li, char *inst_dirp = NULL; char inst_dir[MAXPATHLEN]; char sep; - int suffix_len = 0; - ldbm_instance *inst = NULL; + int src_is_fullpath = 0; + ldbm_instance *inst = NULL; if (!src_dir || '\0' == *src_dir) { @@ -5962,20 +5960,28 @@ dblayer_copy_directory(struct ldbminfo *li, else relative_instance_name++; - inst = ldbm_instance_find_by_name(li, relative_instance_name); - if (NULL == inst) { - LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " - "Instance path %s could be invalid.\n", - relative_instance_name, src_dir, 0); - return return_value; + if (is_fullpath(src_dir)) { + src_is_fullpath = 1; + } + if (is_changelog) { + if (!src_is_fullpath) { + LDAPDebug1Arg(LDAP_DEBUG_ANY, "Changelogdir \"%s\" is not full path; " + "Skipping it.\n", src_dir); + return 0; + } + } else { + inst = ldbm_instance_find_by_name(li, relative_instance_name); + if (NULL == inst) { + LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " + "Instance path %s could be invalid.\n", + relative_instance_name, src_dir, 0); + return return_value; + } } - if (is_fullpath(src_dir)) - { + if (src_is_fullpath) { new_src_dir = src_dir; - } - else - { + } else { int len; inst_dirp = dblayer_get_full_inst_dir(inst->inst_li, inst, @@ -6002,7 +6008,6 @@ dblayer_copy_directory(struct ldbminfo *li, return return_value; } - suffix_len = sizeof(LDBM_SUFFIX) - 1; while (NULL != (direntry = PR_ReadDir(dirhandle, PR_SKIP_DOT | PR_SKIP_DOT_DOT))) { @@ -6016,15 +6021,10 @@ dblayer_copy_directory(struct ldbminfo *li, continue; } - /* Look at the last three characters in the filename */ - filename_length = strlen(direntry->name); - if (filename_length > suffix_len) { - offset = filename_length - suffix_len; - } else { - offset = 0; + compare_piece = PL_strrchr((char *)direntry->name, '.'); + if (NULL == compare_piece) { + compare_piece = (char *)direntry->name; } - compare_piece = (char *)direntry->name + offset; - /* rename .db3 -> .db4 or .db4 -> .db */ if (0 == strcmp(compare_piece, LDBM_FILENAME_SUFFIX) || 0 == strcmp(compare_piece, LDBM_SUFFIX_OLD) || @@ -6124,6 +6124,7 @@ out: /* * Get changelogdir from cn=changelog5,cn=config * The value does not have trailing spaces nor slashes. + * The changelogdir value must be a fullpath. */ static int _dblayer_get_changelogdir(struct ldbminfo *li, char **changelogdir) @@ -6339,7 +6340,7 @@ dblayer_backup(struct ldbminfo *li, char *dest_dir, Slapi_Task *task) return_value = dblayer_copy_directory(li, task, changelogdir, changelog_destdir, 0 /* backup */, - &cnt, 0, 0, 0); + &cnt, 0, 0, 1); if (return_value) { LDAPDebug(LDAP_DEBUG_ANY, "Backup: error in copying directory " @@ -7012,7 +7013,8 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * /* Get the parent dir of changelogdir */ *cldirname = '\0'; return_value = dblayer_copy_directory(li, task, filename1, - changelogdir, 1 /* restore */, &cnt, 0, 0, 0); + changelogdir, 1 /* restore */, + &cnt, 0, 0, 1); *cldirname = '/'; if (return_value) { LDAPDebug1Arg(LDAP_DEBUG_ANY, @@ -7044,7 +7046,7 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * restore_dir = inst->inst_parent_dir_name; /* If we're doing a partial restore, we need to reset the LSNs on the data files */ if (dblayer_copy_directory(li, task, filename1, - restore_dir, 1 /* restore */, &cnt, 0, 0, (bename) ? 1 : 0) == 0) + restore_dir, 1 /* restore */, &cnt, 0, (bename) ? 1 : 0, 0) == 0) continue; else { diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 0022bfd..0fc8025 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -143,8 +143,8 @@ int dblayer_backup(struct ldbminfo *li, char *destination_directory, int dblayer_restore(struct ldbminfo *li, char* source_directory, Slapi_Task *task, char *bename); int dblayer_copy_directory(struct ldbminfo *li, Slapi_Task *task, char *instance_dir, char *destination_dir, - int restore, int *cnt, int instance_dir_flag, - int indexonly, int resetlsns); + int restore, int *cnt, int indexonly, + int resetlsns, int is_changelog); int dblayer_copyfile(char* source, char * destination, int overwrite, int mode); int dblayer_delete_instance_dir(backend *be); int dblayer_delete_database(struct ldbminfo *li);

1 0

ldap/servers
by Noriko Hosoi 16 May '14

16 May '14

ldap/servers/slapd/back-ldbm/dbhelp.c | 6 ++ ldap/servers/slapd/back-ldbm/dblayer.c | 60 ++++++++++++------------- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 4 - 3 files changed, 39 insertions(+), 31 deletions(-) New commits: commit 819cbfd6ee29cc485f516ff4829cf2ff10ce81de Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Thu May 15 18:57:21 2014 -0700 Ticket #47804 - db2bak.pl error with changelogdb Bug description: Backup utility db2bak[.pl] copies not just backend db files but also changelog db files, which is not associated with the backend instance, but the backup code blindly expected it. Fix description: If the copying directory is a changelog db dir, skip retrieving the backend instance info and just copy the files underneath. https://fedorahosted.org/389/ticket/47804 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) diff --git a/ldap/servers/slapd/back-ldbm/dbhelp.c b/ldap/servers/slapd/back-ldbm/dbhelp.c index 29c0247..40966ac 100644 --- a/ldap/servers/slapd/back-ldbm/dbhelp.c +++ b/ldap/servers/slapd/back-ldbm/dbhelp.c @@ -139,6 +139,12 @@ dblayer_copy_file_keybykey(DB_ENV *env, (*(p + sizeof(LDBM_ENTRYRDN_STR) - 1) == '.')) { /* entryrdn.db */ struct attrinfo *ai = NULL; + if (NULL == inst) { + LDAPDebug0Args(LDAP_DEBUG_ANY, + "dblayer_copy_file_keybykey(entryrdn), " + "dup_cmp_fn cannot be retrieved since inst is NULL.\n"); + goto error; + } ainfo_get(inst->inst_be, LDBM_ENTRYRDN_STR, &ai); if (ai->ai_dup_cmp_fn) { /* If set, use the special dup compare callback */ diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c index cb0509f..0fda6d3 100644 --- a/ldap/servers/slapd/back-ldbm/dblayer.c +++ b/ldap/servers/slapd/back-ldbm/dblayer.c @@ -5922,17 +5922,15 @@ dblayer_copy_directory(struct ldbminfo *li, char *dest_dir, int restore, int *cnt, - int instance_dir_flag, int indexonly, - int resetlsns) + int resetlsns, + int is_changelog) { dblayer_private *priv = NULL; char *new_src_dir = NULL; char *new_dest_dir = NULL; PRDir *dirhandle = NULL; PRDirEntry *direntry = NULL; - size_t filename_length = 0; - size_t offset = 0; char *compare_piece = NULL; char *filename1; char *filename2; @@ -5941,8 +5939,8 @@ dblayer_copy_directory(struct ldbminfo *li, char *inst_dirp = NULL; char inst_dir[MAXPATHLEN]; char sep; - int suffix_len = 0; - ldbm_instance *inst = NULL; + int src_is_fullpath = 0; + ldbm_instance *inst = NULL; if (!src_dir || '\0' == *src_dir) { @@ -5966,20 +5964,28 @@ dblayer_copy_directory(struct ldbminfo *li, else relative_instance_name++; - inst = ldbm_instance_find_by_name(li, relative_instance_name); - if (NULL == inst) { - LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " - "Instance path %s could be invalid.\n", - relative_instance_name, src_dir, 0); - return return_value; + if (is_fullpath(src_dir)) { + src_is_fullpath = 1; + } + if (is_changelog) { + if (!src_is_fullpath) { + LDAPDebug1Arg(LDAP_DEBUG_ANY, "Changelogdir \"%s\" is not full path; " + "Skipping it.\n", src_dir); + return 0; + } + } else { + inst = ldbm_instance_find_by_name(li, relative_instance_name); + if (NULL == inst) { + LDAPDebug(LDAP_DEBUG_ANY, "Backend instance \"%s\" does not exist; " + "Instance path %s could be invalid.\n", + relative_instance_name, src_dir, 0); + return return_value; + } } - if (is_fullpath(src_dir)) - { + if (src_is_fullpath) { new_src_dir = src_dir; - } - else - { + } else { int len; inst_dirp = dblayer_get_full_inst_dir(inst->inst_li, inst, @@ -6006,7 +6012,6 @@ dblayer_copy_directory(struct ldbminfo *li, return return_value; } - suffix_len = sizeof(LDBM_SUFFIX) - 1; while (NULL != (direntry = PR_ReadDir(dirhandle, PR_SKIP_DOT | PR_SKIP_DOT_DOT))) { @@ -6020,15 +6025,10 @@ dblayer_copy_directory(struct ldbminfo *li, continue; } - /* Look at the last three characters in the filename */ - filename_length = strlen(direntry->name); - if (filename_length > suffix_len) { - offset = filename_length - suffix_len; - } else { - offset = 0; + compare_piece = PL_strrchr((char *)direntry->name, '.'); + if (NULL == compare_piece) { + compare_piece = (char *)direntry->name; } - compare_piece = (char *)direntry->name + offset; - /* rename .db3 -> .db4 or .db4 -> .db */ if (0 == strcmp(compare_piece, LDBM_FILENAME_SUFFIX) || 0 == strcmp(compare_piece, LDBM_SUFFIX_OLD) || @@ -6128,6 +6128,7 @@ out: /* * Get changelogdir from cn=changelog5,cn=config * The value does not have trailing spaces nor slashes. + * The changelogdir value must be a fullpath. */ static int _dblayer_get_changelogdir(struct ldbminfo *li, char **changelogdir) @@ -6343,7 +6344,7 @@ dblayer_backup(struct ldbminfo *li, char *dest_dir, Slapi_Task *task) return_value = dblayer_copy_directory(li, task, changelogdir, changelog_destdir, 0 /* backup */, - &cnt, 0, 0, 0); + &cnt, 0, 0, 1); if (return_value) { LDAPDebug(LDAP_DEBUG_ANY, "Backup: error in copying directory " @@ -7016,7 +7017,8 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * /* Get the parent dir of changelogdir */ *cldirname = '\0'; return_value = dblayer_copy_directory(li, task, filename1, - changelogdir, 1 /* restore */, &cnt, 0, 0, 0); + changelogdir, 1 /* restore */, + &cnt, 0, 0, 1); *cldirname = '/'; if (return_value) { LDAPDebug1Arg(LDAP_DEBUG_ANY, @@ -7048,7 +7050,7 @@ int dblayer_restore(struct ldbminfo *li, char *src_dir, Slapi_Task *task, char * restore_dir = inst->inst_parent_dir_name; /* If we're doing a partial restore, we need to reset the LSNs on the data files */ if (dblayer_copy_directory(li, task, filename1, - restore_dir, 1 /* restore */, &cnt, 0, 0, (bename) ? 1 : 0) == 0) + restore_dir, 1 /* restore */, &cnt, 0, (bename) ? 1 : 0, 0) == 0) continue; else { diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 561a208..0de0632 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -144,8 +144,8 @@ int dblayer_backup(struct ldbminfo *li, char *destination_directory, int dblayer_restore(struct ldbminfo *li, char* source_directory, Slapi_Task *task, char *bename); int dblayer_copy_directory(struct ldbminfo *li, Slapi_Task *task, char *instance_dir, char *destination_dir, - int restore, int *cnt, int instance_dir_flag, - int indexonly, int resetlsns); + int restore, int *cnt, int indexonly, + int resetlsns, int is_changelog); int dblayer_copyfile(char* source, char * destination, int overwrite, int mode); int dblayer_delete_instance_dir(backend *be); int dblayer_delete_database(struct ldbminfo *li);

1 0

Branch '389-ds-base-1.2.11' - ldap/servers
by Noriko Hosoi 16 May '14

16 May '14

ldap/servers/slapd/back-ldbm/ancestorid.c | 28 +- ldap/servers/slapd/back-ldbm/filterindex.c | 50 ++-- ldap/servers/slapd/back-ldbm/idl.c | 92 ++++---- ldap/servers/slapd/back-ldbm/idl_common.c | 12 - ldap/servers/slapd/back-ldbm/idl_new.c | 8 ldap/servers/slapd/back-ldbm/import-merge.c | 6 ldap/servers/slapd/back-ldbm/import-threads.c | 4 ldap/servers/slapd/back-ldbm/index.c | 22 -- ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 2 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 4 ldap/servers/slapd/back-ldbm/ldbm_search.c | 257 +++++++++++++++---------- ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 23 -- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2 ldap/servers/slapd/back-ldbm/seq.c | 3 ldap/servers/slapd/back-ldbm/vlv.c | 2 15 files changed, 289 insertions(+), 226 deletions(-) New commits: commit bd0fe25aaeea3edda7f4bd0d02d5617791aa058f Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Thu May 15 16:17:00 2014 -0700 Ticket #47780 - Some VLV search request causes memory leaks Fix description: . Modified idl_free interface as follows so that passed idl is cleared with NULL once the IDList is successfully freed. -idl_free(IDList *idl) +idl_free(IDList **idl) This change is used to clean up search candidates when ldbm_back_ search_cleanup (ldbm_search.c) is called as an error return. The cleanup function frees the search candidates when it's not NULL and it's not assigned to sr_candidates field in the search result. This fixes a memory leak when VLV/Sort op fails. . ldbm_back_search_cleanup (ldbm_search.c) calls slapi_send_ldap_result if an ldap error is passed to the function. The logic used to be "if (ldap_result>=LDAP_SUCCESS)", which is based upon that mozldap return codes are all positive. Supporting openldap library, there is a chance to get a negative return code (e.g. LDAP_PARAM_ERROR == -9). This patch supports the negative return codes, as well. . In ldbm_back_search (ldbm_search.c) vlv_filter_candidates could ruturn errors such as and LDAP_TIMELIMIT_EXCEEDED, LDAP_ADMINLIMIT_ EXCEEDED. The search results are supposed to be returned to the client with the error code if the control is not critical. The code is added. . The VLV operation stores the result in vlv_response_control.result in ldbm_back_search (ldbm_search.c), which occurs at 3 places, vlv_ filter_candidates, sort_candidates and vlv_trim_candidates_txn. The return code from the latter calls used to override the former return code. This patch fixes it to respect the former return code. https://fedorahosted.org/389/ticket/47780 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) (cherry picked from commit 1118cc9b61a60c704250080d6af8785e4f8d28af) (cherry picked from commit e9f86dab95fd600b414cf1c4cf6f7c733348b758) (cherry picked from commit ff4c7a7b3f65589f93d88aadfcbe95976a9b31a9) diff --git a/ldap/servers/slapd/back-ldbm/ancestorid.c b/ldap/servers/slapd/back-ldbm/ancestorid.c index 8722b68..54b08ad 100644 --- a/ldap/servers/slapd/back-ldbm/ancestorid.c +++ b/ldap/servers/slapd/back-ldbm/ancestorid.c @@ -136,7 +136,7 @@ static int ldbm_get_nonleaf_ids(backend *be, DB_TXN *txn, IDList **idl) LDAPDebug(LDAP_DEBUG_TRACE, "found %lu nodes for ancestorid\n", (u_long)IDL_NIDS(nodes), 0, 0); } else { - idl_free(nodes); + idl_free(&nodes); *idl = NULL; } @@ -264,7 +264,7 @@ static int ldbm_ancestorid_default_create_index(backend *be) /* Insert into ancestorid for this node */ if (id2idl_hash_lookup(ht, &id, &ididl)) { descendants = idl_union_allids(be, ai_aid, ididl->idl, children); - idl_free(children); + idl_free(&children); if (id2idl_hash_remove(ht, &id) == 0) { LDAPDebug(LDAP_DEBUG_ANY, "ancestorid hash_remove failed\n", 0,0,0); } else { @@ -279,21 +279,21 @@ static int ldbm_ancestorid_default_create_index(backend *be) /* Get parentid for this entry */ ret = ldbm_parentid(be, txn, id, &parentid); if (ret != 0) { - idl_free(descendants); + idl_free(&descendants); break; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(descendants); + idl_free(&descendants); continue; } /* Insert into ancestorid for this node's parent */ if (id2idl_hash_lookup(ht, &parentid, &ididl)) { IDList *idl = idl_union_allids(be, ai_aid, ididl->idl, descendants); - idl_free(descendants); - idl_free(ididl->idl); + idl_free(&descendants); + idl_free(&(ididl->idl)); ididl->idl = idl; } else { ididl = (id2idl*)slapi_ch_calloc(1,sizeof(id2idl)); @@ -324,7 +324,7 @@ static int ldbm_ancestorid_default_create_index(backend *be) id2idl_hash_destroy(ht); /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -456,7 +456,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) /* Insert into ancestorid for this node */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); break; } @@ -467,13 +467,13 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) slapi_log_error(SLAPI_LOG_FATAL, sourcefile, "Error: ldbm_parentid on node index [" ID_FMT "] of [" ID_FMT "]\n", nids, nodes->b_nids); - idl_free(children); + idl_free(&children); goto out; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(children); + idl_free(&children); break; } @@ -485,7 +485,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) /* Insert into ancestorid for this node's parent */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); goto out; } id = parentid; @@ -504,7 +504,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) } /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -583,7 +583,7 @@ static int ldbm_parentid(backend *be, DB_TXN *txn, ID id, ID *ppid) static void id2idl_free(id2idl **ididl) { - idl_free((*ididl)->idl); + idl_free(&((*ididl)->idl)); slapi_ch_free((void**)ididl); } @@ -785,7 +785,7 @@ static int ldbm_ancestorid_index_update( break; } node_id = idl_firstid(idl); - idl_free(idl); + idl_free(&idl); } /* Update ancestorid for the base entry */ diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c index a213815..0998920 100644 --- a/ldap/servers/slapd/back-ldbm/filterindex.c +++ b/ldap/servers/slapd/back-ldbm/filterindex.c @@ -399,7 +399,7 @@ presence_candidates( LDAPDebug(LDAP_DEBUG_TRACE, "fallback to eq index as pres index gave allids\n", 0, 0, 0); - idl_free(idl); + idl_free(&idl); idl = index_range_read_ext(pb, be, type, indextype_EQUALITY, SLAPI_OP_GREATER_OR_EQUAL, NULL, NULL, 0, &txn, err, allidslimit); @@ -481,7 +481,7 @@ extensible_candidates( else if (keys == NULL || keys[0] == NULL) { /* no keys */ - idl_free (idl); + idl_free (&idl); idl = idl_allids (be); } else @@ -516,8 +516,8 @@ extensible_candidates( else { IDList* tmp = idl_intersection (be, idl2, idl3); - idl_free (idl2); - idl_free (idl3); + idl_free (&idl2); + idl_free (&idl3); idl2 = tmp; } if (idl2 == NULL) break; /* look no further */ @@ -529,8 +529,8 @@ extensible_candidates( else if (idl2 != NULL) { IDList* tmp = idl_union (be, idl, idl2); - idl_free (idl); - idl_free (idl2); + idl_free (&idl); + idl_free (&idl2); idl = tmp; } } @@ -797,7 +797,7 @@ list_candidates( { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -807,7 +807,7 @@ list_candidates( == NULL && ftype == LDAP_FILTER_AND ) { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -822,18 +822,24 @@ list_candidates( } } else if ( ftype == LDAP_FILTER_AND ) { if (isnot) { - IDList *new_idl = NULL; - int notin_result = 0; - notin_result = idl_notin( be, idl, tmp, &new_idl ); - if (notin_result) { - idl_free(idl); - idl = new_idl; + /* + * If tmp is NULL or ALLID, idl_notin just duplicates idl. + * We don't have to do it. + */ + if (!tmp && !idl_is_allids(tmp)) { + IDList *new_idl = NULL; + int notin_result = 0; + notin_result = idl_notin( be, idl, tmp, &new_idl ); + if (notin_result) { + idl_free(&idl); + idl = new_idl; + } } } else { idl = idl_intersection(be, idl, tmp); - idl_free( tmp2 ); + idl_free( &tmp2 ); } - idl_free( tmp ); + idl_free( &tmp ); /* stop if the list has gotten too small */ if ((idl == NULL) || (idl_length(idl) <= FILTER_TEST_THRESHOLD)) @@ -843,8 +849,8 @@ list_candidates( slapi_pblock_get( pb, SLAPI_OPERATION, &operation ); idl = idl_union( be, idl, tmp ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); /* stop if we're already committed to an exhaustive * search. :( */ @@ -853,7 +859,7 @@ list_candidates( if (op_is_pagedresults(operation)) { int nids = IDL_NIDS(idl); if ( allidslimit > 0 && nids > allidslimit ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } } @@ -976,7 +982,7 @@ keys2idl( } #endif if ( idl2 == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl = NULL; break; } @@ -988,8 +994,8 @@ keys2idl( tmp = idl; idl = idl_intersection(be, idl, idl2); - idl_free( idl2 ); - idl_free( tmp ); + idl_free( &idl2 ); + idl_free( &tmp ); if ( idl == NULL ) { break; } diff --git a/ldap/servers/slapd/back-ldbm/idl.c b/ldap/servers/slapd/back-ldbm/idl.c index 76ee252..67fa711 100644 --- a/ldap/servers/slapd/back-ldbm/idl.c +++ b/ldap/servers/slapd/back-ldbm/idl.c @@ -280,12 +280,12 @@ idl_old_fetch( if ( ! INDIRECT_BLOCK( idl ) ) { /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); } - idl_free( idl ); + idl_free( &idl ); /* Taking a transaction is expensive; so we try and optimize for the common case by not taking one above. If we have a indirect block; we need to take a transaction and re-read @@ -307,7 +307,7 @@ idl_old_fetch( dblayer_read_txn_commit(li,&s_txn); /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); @@ -367,7 +367,7 @@ idl_old_fetch( dblayer_read_txn_commit(li,&s_txn); tmp[i] = NULL; slapi_ch_free((void**)&kstr ); - idl_free( idl ); + idl_free( &idl ); /* allocate space for the big block */ idl = idl_alloc( nids ); @@ -384,7 +384,7 @@ idl_old_fetch( tmp[i]->b_nids * sizeof(ID) ); nids += tmp[i]->b_nids; - idl_free( tmp[i] ); + idl_free( &tmp[i] ); } slapi_ch_free((void**)&tmp ); @@ -629,7 +629,7 @@ idl_old_insert_key( rc, (msg = dblayer_strerror( rc )) ? msg : "", 0 ); } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -655,10 +655,10 @@ idl_old_insert_key( case 3: /* id not inserted - block must be split */ /* check threshold for marking this an all-id block */ if ( a->ai_idl->idl_maxindirect < 2 ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK) @@ -673,7 +673,7 @@ idl_old_insert_key( } idl_split_block( idl, id, &tmp, &tmp2 ); - idl_free( idl ); + idl_free( &idl ); /* create the header indirect block */ idl = idl_alloc( 3 ); @@ -686,9 +686,9 @@ idl_old_insert_key( /* store it */ rc = idl_store( be, db, key, idl, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 3 BAD %d %s\n", @@ -712,9 +712,9 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 4 BAD %d %s\n", @@ -726,12 +726,12 @@ idl_old_insert_key( "idl_insert_key", "split", key, id); slapi_ch_free((void**)&kstr ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) { @@ -759,7 +759,7 @@ idl_old_insert_key( (u_long)id, key.dptr, i); #endif idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); return( 0 ); } if ( i != 0 ) { @@ -783,7 +783,7 @@ idl_old_insert_key( LDAPDebug( LDAP_DEBUG_ANY, "nonexistent continuation block (%s)\n", k2.dptr, 0, 0 ); idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); slapi_ch_free((void**)&kstr ); return( -1 ); } @@ -911,9 +911,9 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( tmp2 ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); @@ -941,7 +941,7 @@ idl_old_insert_key( * which is not correct. */ rc = 0; - idl_free( tmp2 ); + idl_free( &tmp2 ); break; } if ( rc != 0 ) { @@ -986,7 +986,7 @@ idl_old_insert_key( /* store allid block in place of header block */ if ( 0 == rc ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); if (NULL != disposition) { @@ -996,14 +996,14 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( idl ); - idl_free( tmp ); + idl_free( &idl ); + idl_free( &tmp ); idl_unlock_list(a->ai_idl,key); return( rc ); } idl_split_block( tmp, id, &tmp2, &tmp3 ); - idl_free( tmp ); + idl_free( &tmp ); /* create a new updated indirect header block */ tmp = idl_alloc( idl->b_nmax + 1 ); @@ -1021,8 +1021,8 @@ idl_old_insert_key( /* store the header block */ rc = idl_store( be, db, key, tmp, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1033,8 +1033,8 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1045,22 +1045,22 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp3, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } idl_check_indirect (tmp, i, tmp2, tmp3, "idl_insert_key", "indirect split", key, id); - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -1096,7 +1096,7 @@ int idl_old_store_block( /* If so, store an ALLIDS block */ IDList *all = idl_allids(be); ret = idl_store(be,db,key,all,txn); - idl_free(all); + idl_free(&all); } else { /* Then , is it a block which is smaller than the size at which it needs splitting ? */ if (idl->b_nids <= (ID)priv->idl_maxids) { @@ -1155,7 +1155,7 @@ int idl_old_store_block( make_cont_key(&cont_key,key,lead_id); /* Now store the continuation block */ ret = idl_store(be,db,&cont_key,this_cont_block,txn); - idl_free(this_cont_block); + idl_free(&this_cont_block); slapi_ch_free(&(cont_key.data)); if ( ret != 0 && ret != DB_LOCK_DEADLOCK ) { @@ -1177,7 +1177,7 @@ int idl_old_store_block( } done: /* Free master block */ - idl_free(master_block); + idl_free(&master_block); return ret; } @@ -1438,7 +1438,7 @@ idl_old_delete_key( break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) %d (not indirect)\n", key->dptr, (u_long)id, rc ); @@ -1459,7 +1459,7 @@ idl_old_delete_key( } /* id smaller than smallest id - not there */ if ( i == 0 && id < idl->b_ids[i] ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) -666 (id not found)\n", key->dptr, (u_long)id, 0 ); @@ -1472,7 +1472,7 @@ idl_old_delete_key( /* get the block to delete from */ make_cont_key( &contkey, key, idl->b_ids[i] ); if ( (didl = idl_fetch_one( li, db, &contkey, txn, &rc )) == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != DB_LOCK_DEADLOCK ) { @@ -1559,8 +1559,8 @@ idl_old_delete_key( rc = 0; break; } - idl_free( idl ); - idl_free( didl ); + idl_free( &idl ); + idl_free( &didl ); slapi_ch_free( (void **)&(contkey.dptr) ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) diff --git a/ldap/servers/slapd/back-ldbm/idl_common.c b/ldap/servers/slapd/back-ldbm/idl_common.c index ee59ee2..216bfb0 100644 --- a/ldap/servers/slapd/back-ldbm/idl_common.c +++ b/ldap/servers/slapd/back-ldbm/idl_common.c @@ -85,13 +85,13 @@ idl_allids( backend *be ) } void -idl_free( IDList *idl ) /* JCM - pass in ** */ +idl_free( IDList **idl ) { - if ( idl == NULL ) { + if ((NULL == idl) || (NULL == *idl)) { return; } - slapi_ch_free((void**)&idl ); + slapi_ch_free((void**)idl); } @@ -151,7 +151,7 @@ idl_append_extend(IDList **orig_idl, ID id) /* copy over the existing contents */ idl_new->b_nids = idl->b_nids; memcpy(idl_new->b_ids, idl->b_ids, sizeof(ID) * idl->b_nids); - idl_free(idl); + idl_free(&idl); idl = idl_new; } @@ -243,7 +243,7 @@ idl_intersection( } if ( ni == 0 ) { - idl_free( n ); + idl_free( &n ); return( NULL ); } n->b_nids = ni; @@ -355,7 +355,7 @@ idl_notin( } if ( ni == n->b_nmax ) { - idl_free( n ); + idl_free( &n ); *new_result = idl_allids( be ); } else { n->b_nids = ni; diff --git a/ldap/servers/slapd/back-ldbm/idl_new.c b/ldap/servers/slapd/back-ldbm/idl_new.c index 936c19f..1591e84 100644 --- a/ldap/servers/slapd/back-ldbm/idl_new.c +++ b/ldap/servers/slapd/back-ldbm/idl_new.c @@ -277,7 +277,7 @@ IDList * idl_new_fetch( idl_rc = idl_append_extend(&idl, id); if (idl_rc) { LDAPDebug(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc, 0, 0); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -314,7 +314,7 @@ IDList * idl_new_fetch( idl_rc = idl_append_extend(&idl, id); if (idl_rc) { LDAPDebug(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -330,7 +330,7 @@ IDList * idl_new_fetch( #endif if (ret != DB_NOTFOUND) { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -339,7 +339,7 @@ IDList * idl_new_fetch( /* check for allids value */ if (idl != NULL && idl->b_nids == 1 && idl->b_ids[0] == ALLID) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", key.data, 0, 0); diff --git a/ldap/servers/slapd/back-ldbm/import-merge.c b/ldap/servers/slapd/back-ldbm/import-merge.c index 9d7af3e..e01bf88 100644 --- a/ldap/servers/slapd/back-ldbm/import-merge.c +++ b/ldap/servers/slapd/back-ldbm/import-merge.c @@ -169,8 +169,8 @@ static int import_merge_insert_input_queue(backend *be, import_merge_queue_entry IDList *new_idl = idl_union(be, current_entry->thang.payload.idl, idl); - idl_free(current_entry->thang.payload.idl); - idl_free(idl); + idl_free(&(current_entry->thang.payload.idl)); + idl_free(&idl); current_entry->thang.payload.idl = new_idl; /* Add this file id into the entry's referenced list */ (current_entry->file_referenced_list)[fileno] = 1; @@ -567,7 +567,7 @@ static int import_merge_one_file(ImportWorkerInfo *worker, int passes, ret = idl_store_block(be, output_file, &key, thang.payload.idl, NULL, worker->index_info->ai); /* Free the key we got back from the queue */ - idl_free(thang.payload.idl); + idl_free(&(thang.payload.idl)); thang.payload.idl = NULL; } slapi_ch_free(&(key.data)); diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c index e615bb1..44fd35e 100644 --- a/ldap/servers/slapd/back-ldbm/import-threads.c +++ b/ldap/servers/slapd/back-ldbm/import-threads.c @@ -2005,7 +2005,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) */ if (IDL) { ID id = idl_firstid(IDL); /* entrydn is a single attr */ - idl_free(IDL); + idl_free(&IDL); if (id != entry->ep_id) { /* case (2) */ import_log_notice(job, "Duplicated entrydn detected: \"%s\": " "Entry ID: (%d, %d)", @@ -2031,7 +2031,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) "\"%s\" found at line %d of file \"%s\"", slapi_entry_get_dn(entry->ep_entry), fi->line, fi->filename); - idl_free(IDL); + idl_free(&IDL); /* skip this one */ fi->bad = FIFOITEM_BAD; job->skipped++; diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c index 2d5b535..90d1d23 100644 --- a/ldap/servers/slapd/back-ldbm/index.c +++ b/ldap/servers/slapd/back-ldbm/index.c @@ -197,7 +197,7 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) goto error; } slapi_ch_free( &(bin->key.data) ); - idl_free(bin->value); + idl_free(&(bin->value)); /* If we're already at allids, store an allids block to prevent needless accumulation of blocks */ if (old_idl && ALLIDS(old_idl)) { bin->value = idl_allids(be); @@ -207,10 +207,10 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) } error: if (old_idl) { - idl_free(old_idl); + idl_free(&old_idl); } if (new_idl && need_to_freed_new_idl) { - idl_free(new_idl); + idl_free(&new_idl); } dblayer_release_index_file( be, a, db ); return ret; @@ -268,8 +268,7 @@ index_buffer_terminate(void *h) for (i = 0; i < handle->buffer_size; i++) { bin = &(handle->bins[i]); if (bin->value) { - idl_free(bin->value); - bin->value = NULL; + idl_free(&(bin->value)); } slapi_ch_free(&(bin->key.data)); } @@ -1511,9 +1510,7 @@ index_range_read_ext( */ /* Check to see if we've already looked too hard */ if (idl != NULL && lookthrough_limit != -1 && idl->b_nids > (ID)lookthrough_limit) { - if (NULL != idl) { - idl_free(idl); - } + idl_free(&idl); idl = idl_allids( be ); LDAPDebug(LDAP_DEBUG_TRACE, "index_range_read lookthrough_limit exceeded\n", 0, 0, 0); @@ -1538,10 +1535,7 @@ index_range_read_ext( * when the connection is closed by the client). */ if ( slapi_op_abandoned( pb )) { - if (NULL != idl) { - idl_free(idl); - idl = NULL; - } + idl_free(&idl); LDAPDebug(LDAP_DEBUG_TRACE, "index_range_read - operation abandoned\n", 0, 0, 0); break; /* clean up happens outside the while() loop */ @@ -1577,7 +1571,7 @@ index_range_read_ext( /* idl tmp only contains one id */ /* append it at the end here; sort idlist at the end */ if (ALLIDS(tmp)) { - idl_free(idl); + idl_free(&idl); idl = tmp; } else { ID id; @@ -1588,7 +1582,7 @@ index_range_read_ext( 1097, *err); } } - idl_free(tmp); + idl_free(&tmp); } if (ALLIDS(idl)) { LDAPDebug(LDAP_DEBUG_TRACE, "index_range_read hit an allids value\n", diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c index 38b407d..8369bcc 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c @@ -1090,7 +1090,7 @@ entryrdn_get_subordinates(backend *be, bail: if (rc && subordinates && *subordinates) { - idl_free(*subordinates); + idl_free(subordinates); } slapi_ch_free_string(&keybuf); slapi_ch_free((void **)&elem); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 4e5ce45..09d8f05 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -1451,7 +1451,7 @@ common_return: slapi_ch_free((void**)&child_dns); if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL")) slapi_ch_free((void**)&ldap_result_matcheddn); - idl_free(children); + idl_free(&children); slapi_sdn_done(&dn_newdn); slapi_sdn_done(&dn_newrdn); slapi_sdn_done(&dn_parentdn); @@ -2170,7 +2170,7 @@ moddn_get_children(back_txn *ptxn, } } } while (id!=NOID); - idl_free(candidates); + idl_free(&candidates); slapi_sdn_done(&parentsdn); } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index 746528e..626ba2e 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -46,6 +46,16 @@ #include "back-ldbm.h" #include "vlv_srch.h" +/* + * Used for ldap_result passed to ldbm_back_search_cleanup. + * If (ldap_result == LDBM_SRCH_DEFAULT_RESULT) || (ldap_result == LDAP_SUCCESS), + * don't call slapi_send_ldap_result. + * Note: mozldap ldap_result codes are all positive; openldap result codes could + * be negative values. OL (-1) is LDAP_SERVER_DOWN. Thus, it's safe to + * borrow the value here. + */ +#define LDBM_SRCH_DEFAULT_RESULT (-1) + /* prototypes */ static int build_candidate_list( Slapi_PBlock *pb, backend *be, struct backentry *e, const char * base, int scope, @@ -160,12 +170,14 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, char* ldap_result_description, int function_result, struct vlv_request *vlv_request_control, - struct backentry *e) + struct backentry *e, + IDList *candidates) { int estimate = 0; /* estimated search result count */ backend *be; ldbm_instance *inst; back_search_result_set *sr = NULL; + int free_candidates = 1; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); inst = (ldbm_instance *) be->be_instance_info; @@ -175,20 +187,27 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, { sort_spec_free(sort_control); } - if(ldap_result>=LDAP_SUCCESS) - { + if ((ldap_result != LDBM_SRCH_DEFAULT_RESULT) && (ldap_result != LDAP_SUCCESS)) { slapi_send_ldap_result( pb, ldap_result, NULL, ldap_result_description, 0, NULL ); } /* code to free the result set if we don't need it */ /* We get it and check to see if the structure was ever used */ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr); if (sr) { - if (function_result) { + if (function_result) { /* failed case */ slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate); slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL); + if (sr->sr_candidates == candidates) { + free_candidates = 0; + } delete_search_result_set(pb, &sr); + } else if (sr->sr_candidates == candidates) { /* succeeded case */ + free_candidates = 0; } } + if (free_candidates) { + idl_free(&candidates); + } if (vlv_request_control) { berval_done(&vlv_request_control->value); @@ -331,7 +350,7 @@ ldbm_back_search( Slapi_PBlock *pb ) back_search_result_set *sr = NULL; /* Fix for bugid #394184, SD, 20 Jul 00 */ - int tmp_err = -1; /* must be lower than LDAP_SUCCESS */ + int tmp_err = LDBM_SRCH_DEFAULT_RESULT; char * tmp_desc = NULL; /* end Fix for defect #394184 */ @@ -383,9 +402,29 @@ ldbm_back_search( Slapi_PBlock *pb ) if(r!=0) { /* Badly formed SORT control */ - return ldbm_back_search_cleanup(pb, li, sort_control, - LDAP_PROTOCOL_ERROR, "Sort Control", - SLAPI_FAIL_GENERAL, NULL, NULL); + if (is_sorting_critical_orig) { + /* RFC 4511 4.1.11 the server must not process the operation + * and return LDAP_UNAVAILABLE_CRITICAL_EXTENSION + */ + return ldbm_back_search_cleanup(pb, li, sort_control, + LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "Sort Control", + SLAPI_FAIL_GENERAL, NULL, NULL, candidates); + } else { + PRUint64 conn_id; + int op_id; + + /* Just ignore the control */ + sort = 0; + slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id); + slapi_pblock_get(pb, SLAPI_OPERATION_ID, &op_id); + + LDAPDebug(LDAP_DEBUG_ANY, + "Warning: Sort control ignored for conn=%d op=%d\n", + conn_id, op_id, 0); + } + } else { + /* set this operation includes the server side sorting */ + operation->o_flags |= OP_FLAG_SERVER_SIDE_SORTING; } /* set this operation includes the server side sorting */ operation->o_flags |= OP_FLAG_SERVER_SIDE_SORTING; @@ -401,30 +440,49 @@ ldbm_back_search( Slapi_PBlock *pb ) if(r!=LDAP_SUCCESS) { /* Badly formed VLV control */ - return ldbm_back_search_cleanup(pb, li, sort_control, - r, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); - } - { - /* Access Control Check to see if the client is allowed to use the VLV Control. */ - Slapi_Entry *feature; - char dn[128]; - char *dummyAttr = "dummy#attr"; - char *dummyAttrs[2] = { NULL, NULL }; - - dummyAttrs[0] = dummyAttr; - - /* This dn is normalized. */ - PR_snprintf(dn,sizeof(dn),"dn: oid=%s,cn=features,cn=config",LDAP_CONTROL_VLVREQUEST); - feature= slapi_str2entry(dn,0); - r= plugin_call_acl_plugin (pb, feature, dummyAttrs, NULL, SLAPI_ACL_READ, ACLPLUGIN_ACCESS_DEFAULT, NULL); - slapi_entry_free(feature); - if(r!=LDAP_SUCCESS) + if (is_vlv_critical) { + /* RFC 4511 4.1.11 the server must not process the operation + * and return LDAP_UNAVAILABLE_CRITICAL_EXTENSION + */ + return ldbm_back_search_cleanup(pb, li, sort_control, + LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "VLV Control", SLAPI_FAIL_GENERAL, + &vlv_request_control, NULL, candidates); + } else { + PRUint64 conn_id; + int op_id; + + /* Just ignore the control */ + virtual_list_view = 0; + slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id); + slapi_pblock_get(pb, SLAPI_OPERATION_ID, &op_id); + + LDAPDebug(LDAP_DEBUG_ANY, + "Warning: VLV control ignored for conn=%d op=%d\n", + conn_id, op_id, 0); + } + + } else { { - /* Client isn't allowed to do this. */ - return ldbm_back_search_cleanup(pb, li, sort_control, - r, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + /* Access Control Check to see if the client is allowed to use the VLV Control. */ + Slapi_Entry *feature; + int rc; + char dn[128]; + char *dummyAttr = "dummy#attr"; + char *dummyAttrs[2] = {NULL, NULL}; + + dummyAttrs[0] = dummyAttr; + + /* This dn is normalized. */ + PR_snprintf(dn, sizeof (dn), "dn: oid=%s,cn=features,cn=config", LDAP_CONTROL_VLVREQUEST); + feature = slapi_str2entry(dn, 0); + rc = plugin_call_acl_plugin(pb, feature, dummyAttrs, NULL, SLAPI_ACL_READ, ACLPLUGIN_ACCESS_DEFAULT, NULL); + slapi_entry_free(feature); + if (rc != LDAP_SUCCESS) { + /* Client isn't allowed to do this. */ + return ldbm_back_search_cleanup(pb, li, sort_control, + rc, "VLV Control", SLAPI_FAIL_GENERAL, + &vlv_request_control, NULL, candidates); + } } } /* @@ -438,7 +496,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Can't have a VLV control without a SORT control */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_SORT_CONTROL_MISSING, "VLV Control", - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } } @@ -491,13 +549,13 @@ ldbm_back_search( Slapi_PBlock *pb ) { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNWILLING_TO_PERFORM, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_VIRTUAL_LIST_VIEW_ERROR, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } else @@ -512,7 +570,7 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_make_sort_response_control(pb, LDAP_UNWILLING_TO_PERFORM, NULL); return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else /* vlv and sorting are not critical, so ignore the control */ { @@ -546,7 +604,7 @@ ldbm_back_search( Slapi_PBlock *pb ) { /* error or referral sent by find_entry */ return ldbm_back_search_cleanup(pb, li, sort_control, - -1, NULL, 1, &vlv_request_control, NULL); + LDBM_SRCH_DEFAULT_RESULT, NULL, 1, &vlv_request_control, NULL, candidates); } } @@ -569,6 +627,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* * Build a list of IDs for this entry and scope */ + vlv_response_control.result = LDAP_SUCCESS; if ((NULL != controls) && (sort) && (vlv)) { /* This candidate list is for vlv, no need for sort only. */ switch (vlv_search_build_candidate_list(pb, basesdn, &vlv_rc, @@ -579,12 +638,12 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case VLV_BLD_LIST_FAILED: return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_SUCCESS: /* Log to the access log the particulars of this sort request */ @@ -605,7 +664,7 @@ ldbm_back_search( Slapi_PBlock *pb ) LDAP_OPERATIONS_ERROR, "Sort Response Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -616,9 +675,9 @@ ldbm_back_search( Slapi_PBlock *pb ) if (rc) { /* Error result sent by build_candidate_list */ - return ldbm_back_search_cleanup(pb, li, sort_control, -1, - NULL, rc, - &vlv_request_control, e); + return ldbm_back_search_cleanup(pb, li, sort_control, + LDBM_SRCH_DEFAULT_RESULT, NULL, rc, + &vlv_request_control, e, candidates); } /* * If we're sorting then we must check what administrative @@ -644,25 +703,33 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then apply the * search filter before sorting. */ - if (virtual_list_view && (NULL != candidates)) - { - int r = 0; + if (virtual_list_view && candidates) { IDList *idl = NULL; Slapi_Filter *filter = NULL; slapi_pblock_get( pb, SLAPI_SEARCH_FILTER, &filter ); - r = vlv_filter_candidates(be, pb, candidates, basesdn, - scope, filter, &idl, - lookthrough_limit, time_up); - if(r == 0) - { - idl_free(candidates); - candidates= idl; - } - else - { + rc = vlv_filter_candidates(be, pb, candidates, basesdn, + scope, filter, &idl, + lookthrough_limit, time_up); + switch (rc) { + case LDAP_SUCCESS: /* Everything OK */ + case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ + case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ + vlv_response_control.result = rc; + idl_free(&candidates); + candidates = idl; + break; + case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ + default: return ldbm_back_search_cleanup(pb, li, sort_control, - r, NULL, -1, - &vlv_request_control, e); + rc, NULL, -1, + &vlv_request_control, e, candidates); + } + if (is_vlv_critical && rc) { + idl_free(&candidates); + candidates = idl_alloc(0); + tmp_err = rc; + tmp_desc = "VLV Response Control"; + goto vlv_bail; } } /* @@ -680,7 +747,7 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } else @@ -714,21 +781,24 @@ ldbm_back_search( Slapi_PBlock *pb ) /* replace the hard coded return value by the appropriate * LDAP error code */ switch (sort_return_value) { - case LDAP_SUCCESS: /* Everything OK */ - vlv_response_control.result= LDAP_SUCCESS; + case LDAP_SUCCESS: + /* + * we don't want to override an error from vlv + * vlv_response_control.result= LDAP_SUCCESS; + */ break; case LDAP_PROTOCOL_ERROR: /* A protocol error */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ case LDAP_OPERATIONS_ERROR: /* Operation error */ case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ - vlv_response_control.result= LDAP_TIMELIMIT_EXCEEDED; - break; case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ - vlv_response_control.result = LDAP_ADMINLIMIT_EXCEEDED; + if (!vlv_response_control.result) { + vlv_response_control.result = sort_return_value; + } break; case LDAP_OTHER: /* Abandoned */ abandoned = 1; /* So that we don't return a result code */ @@ -747,11 +817,10 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Fix for bugid #394184, SD, 05 Jul 00 */ /* we were not actually returning unavailableCriticalExtension; now fixed (hopefully !) */ - if (is_sorting_critical && (0 != sort_return_value)) - { - idl_free(candidates); + if (is_sorting_critical && sort_return_value) { + idl_free(&candidates); candidates = idl_alloc(0); - tmp_err = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + tmp_err = sort_return_value; tmp_desc = "Sort Response Control"; } /* end Fix for bugid #394184 */ @@ -761,9 +830,9 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_return_value, sort_error_type ) ) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -771,10 +840,9 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then the candidate list * must be trimmed down to just the range of entries requested. */ - if (virtual_list_view) - { - if (NULL != candidates && candidates->b_nids>0) - { + if (virtual_list_view) { + if (candidates && (candidates->b_nids > 0) && + !vlv_response_control.result) { IDList *idl= NULL; back_txn txn = {NULL}; slapi_pblock_get(pb, SLAPI_TXN, &txn.back_txn_txn); @@ -783,7 +851,7 @@ ldbm_back_search( Slapi_PBlock *pb ) &vlv_request_control, &idl, &vlv_response_control, &txn); if(vlv_response_control.result==0) { - idl_free(candidates); + idl_free(&candidates); candidates = idl; } else @@ -791,26 +859,23 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } - } - else - { + } else { vlv_response_control.targetPosition = 0; vlv_response_control.contentCount = 0; - vlv_response_control.result = LDAP_SUCCESS; + /* vlv_response_control.result = LDAP_SUCCESS; Don't override */ } } } - if (virtual_list_view) - { - if(LDAP_SUCCESS != - vlv_make_response_control( pb, &vlv_response_control )) - { +vlv_bail: + if (virtual_list_view) { + if (LDAP_SUCCESS != + vlv_make_response_control( pb, &vlv_response_control )) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "VLV Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } /* Log the VLV operation */ vlv_print_access_log(pb,&vlv_request_control,&vlv_response_control); @@ -840,7 +905,7 @@ ldbm_back_search( Slapi_PBlock *pb ) PR_Unlock(inst->inst_config_mutex); if (ri) { - idl_free(candidates); + idl_free(&candidates); candidates = idl_alloc(0); tmp_err = LDAP_UNWILLING_TO_PERFORM; tmp_desc = "Search is not indexed"; @@ -903,10 +968,10 @@ ldbm_back_search( Slapi_PBlock *pb ) } /* Fix for bugid #394184, SD, 05 Jul 00 */ - /* tmp_err == -1: no error */ + /* tmp_err == LDBM_SRCH_DEFAULT_RESULT: no error */ return ldbm_back_search_cleanup(pb, li, sort_control, tmp_err, tmp_desc, - (tmp_err == -1 ? 0 : -1), - &vlv_request_control, NULL); + (tmp_err == LDBM_SRCH_DEFAULT_RESULT ? 0 : LDBM_SRCH_DEFAULT_RESULT), + &vlv_request_control, NULL, candidates); /* end Fix for bugid #394184 */ } @@ -1167,14 +1232,14 @@ subtree_candidates( e->ep_id, &descendants, &txn, 0); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } else if (!has_tombstone_filter) { *err = ldbm_ancestorid_read_ext(be, &txn, e->ep_id, &descendants, allidslimit); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } /* else == has_tombstone_filter: do nothing */ } @@ -1802,7 +1867,7 @@ delete_search_result_set( Slapi_PBlock *pb, back_search_result_set **sr ) } if ( NULL != (*sr)->sr_candidates ) { - idl_free( (*sr)->sr_candidates ); + idl_free( &((*sr)->sr_candidates) ); } rc = slapi_filter_apply((*sr)->sr_norm_filter, ldbm_search_free_compiled_filter, NULL, &filt_errs); diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c index 51ff62b..76eea2c 100644 --- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c +++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c @@ -291,7 +291,7 @@ int add_op_attrs(Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *ep, if ( (idl = index_read( be, LDBM_ENTRYDN_STR, indextype_EQUALITY, &bv, NULL, &err )) != NULL ) { pid = idl_firstid( idl ); - idl_free( idl ); + idl_free( &idl ); } else { /* empty idl */ if ( 0 != err && DB_NOTFOUND != err ) { @@ -591,7 +591,7 @@ int update_subordinatecounts(backend *be, import_subcount_stuff *mothers, /* We get the count from the IDL */ sub_count = idl->b_nids; } - idl_free(idl); + idl_free(&idl); } /* Did we get the count ? */ if (found_count) { @@ -889,8 +889,7 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) continue; } id = idl_firstid(idl); - idl_free(idl); - idl = NULL; + idl_free(&idl); } /* @@ -928,8 +927,8 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) idltotal = idl; } else if (idl) { idltmp = idl_union(be, idltotal, idl); - idl_free(idltotal); - idl_free(idl); + idl_free(&idltotal); + idl_free(&idl); idltotal = idltmp; } } /* for (i = 0; include[i]; i++) */ @@ -1331,8 +1330,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) /* allids list is no help at all -- revert to trawling * the whole list. */ ok_index = 0; - idl_free(idl); - idl = NULL; + idl_free(&idl); } idindex = 0; } @@ -1588,7 +1586,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) } bye: if (idl) { - idl_free(idl); + idl_free(&idl); } if (dbc) { dbc->c_close(dbc); @@ -1619,7 +1617,7 @@ bye: } ldbm_back_free_incl_excl(include_suffix, exclude_suffix); - idl_free(eargs.pre_exported_idl); + idl_free(&(eargs.pre_exported_idl)); return( return_value ); } @@ -1933,8 +1931,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) } } else if (ALLIDS(idl)) { /* that's no help. */ - idl_free(idl); - idl = NULL; + idl_free(&idl); } } @@ -2443,7 +2440,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) err_out: backentry_free( &ep ); /* if ep or *ep is NULL, it does nothing */ if (idl) { - idl_free(idl); + idl_free(&idl); } else { dbc->c_close(dbc); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index f3c84a1..499c92b 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -232,7 +232,7 @@ struct backentry * id2entry( backend *be, ID id, back_txn *txn, int *err ); * idl.c */ IDList * idl_alloc( NIDS nids ); -void idl_free( IDList *idl ); +void idl_free( IDList **idl ); NIDS idl_length(IDList *idl); int idl_is_allids(IDList *idl); int idl_append(IDList *idl, ID id); diff --git a/ldap/servers/slapd/back-ldbm/seq.c b/ldap/servers/slapd/back-ldbm/seq.c index ab473bd..a947438 100644 --- a/ldap/servers/slapd/back-ldbm/seq.c +++ b/ldap/servers/slapd/back-ldbm/seq.c @@ -235,6 +235,7 @@ ldbm_back_seq( Slapi_PBlock *pb ) key.flags = 0; for (retry_count = 0; retry_count < IDL_FETCH_RETRY_COUNT; retry_count++) { err = NEW_IDL_DEFAULT; + idl_free(&idl); idl = idl_fetch( be, db, &key, NULL, ai, &err ); if(err == DB_LOCK_DEADLOCK) { ldbm_nasty("ldbm_back_seq deadlock retry", 1600, err); @@ -289,7 +290,7 @@ ldbm_back_seq( Slapi_PBlock *pb ) } CACHE_RETURN( &inst->inst_cache, &e ); } - idl_free( idl ); + idl_free( &idl ); } dblayer_release_index_file( be, ai, db ); diff --git a/ldap/servers/slapd/back-ldbm/vlv.c b/ldap/servers/slapd/back-ldbm/vlv.c index dcc42fa..ad38889 100644 --- a/ldap/servers/slapd/back-ldbm/vlv.c +++ b/ldap/servers/slapd/back-ldbm/vlv.c @@ -1112,7 +1112,7 @@ int vlv_build_idl(PRUint32 start, PRUint32 stop, DB *db, DBC *dbc, goto done; error: - if (idl) idl_free(idl); + if (idl) idl_free(&idl); done: return rc;

1 0

Branch '389-ds-base-1.3.1' - ldap/servers
by Noriko Hosoi 15 May '14

15 May '14

ldap/servers/slapd/back-ldbm/ancestorid.c | 28 ++-- ldap/servers/slapd/back-ldbm/filterindex.c | 50 ++++--- ldap/servers/slapd/back-ldbm/idl.c | 92 ++++++------- ldap/servers/slapd/back-ldbm/idl_common.c | 12 - ldap/servers/slapd/back-ldbm/idl_new.c | 20 +-- ldap/servers/slapd/back-ldbm/import-merge.c | 6 ldap/servers/slapd/back-ldbm/import-threads.c | 4 ldap/servers/slapd/back-ldbm/index.c | 16 +- ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 2 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 4 ldap/servers/slapd/back-ldbm/ldbm_search.c | 166 ++++++++++++++----------- ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 23 +-- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2 ldap/servers/slapd/back-ldbm/seq.c | 4 ldap/servers/slapd/back-ldbm/vlv.c | 2 15 files changed, 231 insertions(+), 200 deletions(-) New commits: commit ff4c7a7b3f65589f93d88aadfcbe95976a9b31a9 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Thu May 15 14:43:28 2014 -0700 Ticket #47780 - Some VLV search request causes memory leaks Fix description: . Modified idl_free interface as follows so that passed idl is cleared with NULL once the IDList is successfully freed. -idl_free(IDList *idl) +idl_free(IDList **idl) This change is used to clean up search candidates when ldbm_back_ search_cleanup (ldbm_search.c) is called as an error return. The cleanup function frees the search candidates when it's not NULL and it's not assigned to sr_candidates field in the search result. This fixes a memory leak when VLV/Sort op fails. . ldbm_back_search_cleanup (ldbm_search.c) calls slapi_send_ldap_result if an ldap error is passed to the function. The logic used to be "if (ldap_result>=LDAP_SUCCESS)", which is based upon that mozldap return codes are all positive. Supporting openldap library, there is a chance to get a negative return code (e.g. LDAP_PARAM_ERROR == -9). This patch supports the negative return codes, as well. . In ldbm_back_search (ldbm_search.c) vlv_filter_candidates could ruturn errors such as and LDAP_TIMELIMIT_EXCEEDED, LDAP_ADMINLIMIT_ EXCEEDED. The search results are supposed to be returned to the client with the error code if the control is not critical. The code is added. . The VLV operation stores the result in vlv_response_control.result in ldbm_back_search (ldbm_search.c), which occurs at 3 places, vlv_ filter_candidates, sort_candidates and vlv_trim_candidates_txn. The return code from the latter calls used to override the former return code. This patch fixes it to respect the former return code. https://fedorahosted.org/389/ticket/47780 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) (cherry picked from commit 1118cc9b61a60c704250080d6af8785e4f8d28af) (cherry picked from commit e9f86dab95fd600b414cf1c4cf6f7c733348b758) diff --git a/ldap/servers/slapd/back-ldbm/ancestorid.c b/ldap/servers/slapd/back-ldbm/ancestorid.c index 8722b68..54b08ad 100644 --- a/ldap/servers/slapd/back-ldbm/ancestorid.c +++ b/ldap/servers/slapd/back-ldbm/ancestorid.c @@ -136,7 +136,7 @@ static int ldbm_get_nonleaf_ids(backend *be, DB_TXN *txn, IDList **idl) LDAPDebug(LDAP_DEBUG_TRACE, "found %lu nodes for ancestorid\n", (u_long)IDL_NIDS(nodes), 0, 0); } else { - idl_free(nodes); + idl_free(&nodes); *idl = NULL; } @@ -264,7 +264,7 @@ static int ldbm_ancestorid_default_create_index(backend *be) /* Insert into ancestorid for this node */ if (id2idl_hash_lookup(ht, &id, &ididl)) { descendants = idl_union_allids(be, ai_aid, ididl->idl, children); - idl_free(children); + idl_free(&children); if (id2idl_hash_remove(ht, &id) == 0) { LDAPDebug(LDAP_DEBUG_ANY, "ancestorid hash_remove failed\n", 0,0,0); } else { @@ -279,21 +279,21 @@ static int ldbm_ancestorid_default_create_index(backend *be) /* Get parentid for this entry */ ret = ldbm_parentid(be, txn, id, &parentid); if (ret != 0) { - idl_free(descendants); + idl_free(&descendants); break; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(descendants); + idl_free(&descendants); continue; } /* Insert into ancestorid for this node's parent */ if (id2idl_hash_lookup(ht, &parentid, &ididl)) { IDList *idl = idl_union_allids(be, ai_aid, ididl->idl, descendants); - idl_free(descendants); - idl_free(ididl->idl); + idl_free(&descendants); + idl_free(&(ididl->idl)); ididl->idl = idl; } else { ididl = (id2idl*)slapi_ch_calloc(1,sizeof(id2idl)); @@ -324,7 +324,7 @@ static int ldbm_ancestorid_default_create_index(backend *be) id2idl_hash_destroy(ht); /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -456,7 +456,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) /* Insert into ancestorid for this node */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); break; } @@ -467,13 +467,13 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) slapi_log_error(SLAPI_LOG_FATAL, sourcefile, "Error: ldbm_parentid on node index [" ID_FMT "] of [" ID_FMT "]\n", nids, nodes->b_nids); - idl_free(children); + idl_free(&children); goto out; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(children); + idl_free(&children); break; } @@ -485,7 +485,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) /* Insert into ancestorid for this node's parent */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); goto out; } id = parentid; @@ -504,7 +504,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) } /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -583,7 +583,7 @@ static int ldbm_parentid(backend *be, DB_TXN *txn, ID id, ID *ppid) static void id2idl_free(id2idl **ididl) { - idl_free((*ididl)->idl); + idl_free(&((*ididl)->idl)); slapi_ch_free((void**)ididl); } @@ -785,7 +785,7 @@ static int ldbm_ancestorid_index_update( break; } node_id = idl_firstid(idl); - idl_free(idl); + idl_free(&idl); } /* Update ancestorid for the base entry */ diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c index d1fddf9..8a52d0d 100644 --- a/ldap/servers/slapd/back-ldbm/filterindex.c +++ b/ldap/servers/slapd/back-ldbm/filterindex.c @@ -399,7 +399,7 @@ presence_candidates( LDAPDebug(LDAP_DEBUG_TRACE, "fallback to eq index as pres index gave allids\n", 0, 0, 0); - idl_free(idl); + idl_free(&idl); idl = index_range_read_ext(pb, be, type, indextype_EQUALITY, SLAPI_OP_GREATER_OR_EQUAL, NULL, NULL, 0, &txn, err, allidslimit); @@ -481,7 +481,7 @@ extensible_candidates( else if (keys == NULL || keys[0] == NULL) { /* no keys */ - idl_free (idl); + idl_free (&idl); idl = idl_allids (be); } else @@ -516,8 +516,8 @@ extensible_candidates( else { IDList* tmp = idl_intersection (be, idl2, idl3); - idl_free (idl2); - idl_free (idl3); + idl_free (&idl2); + idl_free (&idl3); idl2 = tmp; } if (idl2 == NULL) break; /* look no further */ @@ -529,8 +529,8 @@ extensible_candidates( else if (idl2 != NULL) { IDList* tmp = idl_union (be, idl, idl2); - idl_free (idl); - idl_free (idl2); + idl_free (&idl); + idl_free (&idl2); idl = tmp; } } @@ -797,7 +797,7 @@ list_candidates( { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -807,7 +807,7 @@ list_candidates( == NULL && ftype == LDAP_FILTER_AND ) { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -822,18 +822,24 @@ list_candidates( } } else if ( ftype == LDAP_FILTER_AND ) { if (isnot) { - IDList *new_idl = NULL; - int notin_result = 0; - notin_result = idl_notin( be, idl, tmp, &new_idl ); - if (notin_result) { - idl_free(idl); - idl = new_idl; + /* + * If tmp is NULL or ALLID, idl_notin just duplicates idl. + * We don't have to do it. + */ + if (!tmp && !idl_is_allids(tmp)) { + IDList *new_idl = NULL; + int notin_result = 0; + notin_result = idl_notin( be, idl, tmp, &new_idl ); + if (notin_result) { + idl_free(&idl); + idl = new_idl; + } } } else { idl = idl_intersection(be, idl, tmp); - idl_free( tmp2 ); + idl_free( &tmp2 ); } - idl_free( tmp ); + idl_free( &tmp ); /* stop if the list has gotten too small */ if ((idl == NULL) || (idl_length(idl) <= FILTER_TEST_THRESHOLD)) @@ -843,8 +849,8 @@ list_candidates( slapi_pblock_get( pb, SLAPI_OPERATION, &operation ); idl = idl_union( be, idl, tmp ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); /* stop if we're already committed to an exhaustive * search. :( */ @@ -853,7 +859,7 @@ list_candidates( if (op_is_pagedresults(operation)) { int nids = IDL_NIDS(idl); if ( allidslimit > 0 && nids > allidslimit ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } } @@ -976,7 +982,7 @@ keys2idl( } #endif if ( idl2 == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl = NULL; break; } @@ -988,8 +994,8 @@ keys2idl( tmp = idl; idl = idl_intersection(be, idl, idl2); - idl_free( idl2 ); - idl_free( tmp ); + idl_free( &idl2 ); + idl_free( &tmp ); if ( idl == NULL ) { break; } diff --git a/ldap/servers/slapd/back-ldbm/idl.c b/ldap/servers/slapd/back-ldbm/idl.c index 4309708..3d4e342 100644 --- a/ldap/servers/slapd/back-ldbm/idl.c +++ b/ldap/servers/slapd/back-ldbm/idl.c @@ -280,12 +280,12 @@ idl_old_fetch( if ( ! INDIRECT_BLOCK( idl ) ) { /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); } - idl_free( idl ); + idl_free( &idl ); /* Taking a transaction is expensive; so we try and optimize for the common case by not taking one above. If we have a indirect block; we need to take a transaction and re-read @@ -307,7 +307,7 @@ idl_old_fetch( dblayer_read_txn_commit(be, &s_txn); /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); @@ -367,7 +367,7 @@ idl_old_fetch( dblayer_read_txn_commit(be, &s_txn); tmp[i] = NULL; slapi_ch_free((void**)&kstr ); - idl_free( idl ); + idl_free( &idl ); /* allocate space for the big block */ idl = idl_alloc( nids ); @@ -384,7 +384,7 @@ idl_old_fetch( tmp[i]->b_nids * sizeof(ID) ); nids += tmp[i]->b_nids; - idl_free( tmp[i] ); + idl_free( &tmp[i] ); } slapi_ch_free((void**)&tmp ); @@ -629,7 +629,7 @@ idl_old_insert_key( rc, (msg = dblayer_strerror( rc )) ? msg : "", 0 ); } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -655,10 +655,10 @@ idl_old_insert_key( case 3: /* id not inserted - block must be split */ /* check threshold for marking this an all-id block */ if ( a->ai_idl->idl_maxindirect < 2 ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK) @@ -673,7 +673,7 @@ idl_old_insert_key( } idl_split_block( idl, id, &tmp, &tmp2 ); - idl_free( idl ); + idl_free( &idl ); /* create the header indirect block */ idl = idl_alloc( 3 ); @@ -686,9 +686,9 @@ idl_old_insert_key( /* store it */ rc = idl_store( be, db, key, idl, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 3 BAD %d %s\n", @@ -712,9 +712,9 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 4 BAD %d %s\n", @@ -726,12 +726,12 @@ idl_old_insert_key( "idl_insert_key", "split", key, id); slapi_ch_free((void**)&kstr ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) { @@ -759,7 +759,7 @@ idl_old_insert_key( (u_long)id, key.dptr, i); #endif idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); return( 0 ); } if ( i != 0 ) { @@ -783,7 +783,7 @@ idl_old_insert_key( LDAPDebug( LDAP_DEBUG_ANY, "nonexistent continuation block (%s)\n", k2.dptr, 0, 0 ); idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); slapi_ch_free((void**)&kstr ); return( -1 ); } @@ -911,9 +911,9 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( tmp2 ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); @@ -941,7 +941,7 @@ idl_old_insert_key( * which is not correct. */ rc = 0; - idl_free( tmp2 ); + idl_free( &tmp2 ); break; } if ( rc != 0 ) { @@ -986,7 +986,7 @@ idl_old_insert_key( /* store allid block in place of header block */ if ( 0 == rc ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); if (NULL != disposition) { @@ -996,14 +996,14 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( idl ); - idl_free( tmp ); + idl_free( &idl ); + idl_free( &tmp ); idl_unlock_list(a->ai_idl,key); return( rc ); } idl_split_block( tmp, id, &tmp2, &tmp3 ); - idl_free( tmp ); + idl_free( &tmp ); /* create a new updated indirect header block */ tmp = idl_alloc( idl->b_nmax + 1 ); @@ -1021,8 +1021,8 @@ idl_old_insert_key( /* store the header block */ rc = idl_store( be, db, key, tmp, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1033,8 +1033,8 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1045,22 +1045,22 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp3, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } idl_check_indirect (tmp, i, tmp2, tmp3, "idl_insert_key", "indirect split", key, id); - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -1096,7 +1096,7 @@ int idl_old_store_block( /* If so, store an ALLIDS block */ IDList *all = idl_allids(be); ret = idl_store(be,db,key,all,txn); - idl_free(all); + idl_free(&all); } else { /* Then , is it a block which is smaller than the size at which it needs splitting ? */ if (idl->b_nids <= (ID)priv->idl_maxids) { @@ -1155,7 +1155,7 @@ int idl_old_store_block( make_cont_key(&cont_key,key,lead_id); /* Now store the continuation block */ ret = idl_store(be,db,&cont_key,this_cont_block,txn); - idl_free(this_cont_block); + idl_free(&this_cont_block); slapi_ch_free(&(cont_key.data)); if ( ret != 0 && ret != DB_LOCK_DEADLOCK ) { @@ -1177,7 +1177,7 @@ int idl_old_store_block( } done: /* Free master block */ - idl_free(master_block); + idl_free(&master_block); return ret; } @@ -1438,7 +1438,7 @@ idl_old_delete_key( break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) %d (not indirect)\n", key->dptr, (u_long)id, rc ); @@ -1459,7 +1459,7 @@ idl_old_delete_key( } /* id smaller than smallest id - not there */ if ( i == 0 && id < idl->b_ids[i] ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) -666 (id not found)\n", key->dptr, (u_long)id, 0 ); @@ -1472,7 +1472,7 @@ idl_old_delete_key( /* get the block to delete from */ make_cont_key( &contkey, key, idl->b_ids[i] ); if ( (didl = idl_fetch_one( li, db, &contkey, txn, &rc )) == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != DB_LOCK_DEADLOCK ) { @@ -1559,8 +1559,8 @@ idl_old_delete_key( rc = 0; break; } - idl_free( idl ); - idl_free( didl ); + idl_free( &idl ); + idl_free( &didl ); slapi_ch_free( (void **)&(contkey.dptr) ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) diff --git a/ldap/servers/slapd/back-ldbm/idl_common.c b/ldap/servers/slapd/back-ldbm/idl_common.c index ee59ee2..216bfb0 100644 --- a/ldap/servers/slapd/back-ldbm/idl_common.c +++ b/ldap/servers/slapd/back-ldbm/idl_common.c @@ -85,13 +85,13 @@ idl_allids( backend *be ) } void -idl_free( IDList *idl ) /* JCM - pass in ** */ +idl_free( IDList **idl ) { - if ( idl == NULL ) { + if ((NULL == idl) || (NULL == *idl)) { return; } - slapi_ch_free((void**)&idl ); + slapi_ch_free((void**)idl); } @@ -151,7 +151,7 @@ idl_append_extend(IDList **orig_idl, ID id) /* copy over the existing contents */ idl_new->b_nids = idl->b_nids; memcpy(idl_new->b_ids, idl->b_ids, sizeof(ID) * idl->b_nids); - idl_free(idl); + idl_free(&idl); idl = idl_new; } @@ -243,7 +243,7 @@ idl_intersection( } if ( ni == 0 ) { - idl_free( n ); + idl_free( &n ); return( NULL ); } n->b_nids = ni; @@ -355,7 +355,7 @@ idl_notin( } if ( ni == n->b_nmax ) { - idl_free( n ); + idl_free( &n ); *new_result = idl_allids( be ); } else { n->b_nids = ni; diff --git a/ldap/servers/slapd/back-ldbm/idl_new.c b/ldap/servers/slapd/back-ldbm/idl_new.c index f0410f9..120153e 100644 --- a/ldap/servers/slapd/back-ldbm/idl_new.c +++ b/ldap/servers/slapd/back-ldbm/idl_new.c @@ -285,7 +285,7 @@ idl_new_fetch( idl_rc = idl_append_extend(&idl, id); if (idl_rc) { LDAPDebug(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc, 0, 0); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -323,7 +323,7 @@ idl_new_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -339,7 +339,7 @@ idl_new_fetch( #endif if (ret != DB_NOTFOUND) { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -348,7 +348,7 @@ idl_new_fetch( /* check for allids value */ if (idl != NULL && idl->b_nids == 1 && idl->b_ids[0] == ALLID) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", key.data, 0, 0); @@ -501,7 +501,7 @@ idl_new_range_fetch( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "idl_new_range_fetch - lookthrough_limit exceeded\n"); @@ -557,7 +557,7 @@ idl_new_range_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -618,7 +618,7 @@ idl_new_range_fetch( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "idl_new_range_fetch - lookthrough_limit exceeded\n"); @@ -669,7 +669,7 @@ idl_new_range_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -688,7 +688,7 @@ idl_new_range_fetch( if (ret == DB_NOTFOUND) { ret = 0; /* normal case */ } else { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -696,7 +696,7 @@ idl_new_range_fetch( /* check for allids value */ if (idl && (idl->b_nids == 1) && (idl->b_ids[0] == ALLID)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug1Arg(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", cur_key.data); diff --git a/ldap/servers/slapd/back-ldbm/import-merge.c b/ldap/servers/slapd/back-ldbm/import-merge.c index 223f34f..a728165 100644 --- a/ldap/servers/slapd/back-ldbm/import-merge.c +++ b/ldap/servers/slapd/back-ldbm/import-merge.c @@ -169,8 +169,8 @@ static int import_merge_insert_input_queue(backend *be, import_merge_queue_entry IDList *new_idl = idl_union(be, current_entry->thang.payload.idl, idl); - idl_free(current_entry->thang.payload.idl); - idl_free(idl); + idl_free(&(current_entry->thang.payload.idl)); + idl_free(&idl); current_entry->thang.payload.idl = new_idl; /* Add this file id into the entry's referenced list */ (current_entry->file_referenced_list)[fileno] = 1; @@ -567,7 +567,7 @@ static int import_merge_one_file(ImportWorkerInfo *worker, int passes, ret = idl_store_block(be, output_file, &key, thang.payload.idl, NULL, worker->index_info->ai); /* Free the key we got back from the queue */ - idl_free(thang.payload.idl); + idl_free(&(thang.payload.idl)); thang.payload.idl = NULL; } slapi_ch_free(&(key.data)); diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c index c0eddd9..a26d4ec 100644 --- a/ldap/servers/slapd/back-ldbm/import-threads.c +++ b/ldap/servers/slapd/back-ldbm/import-threads.c @@ -2354,7 +2354,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) */ if (IDL) { ID id = idl_firstid(IDL); /* entrydn is a single attr */ - idl_free(IDL); + idl_free(&IDL); if (id != entry->ep_id) { /* case (2) */ import_log_notice(job, "Duplicated entrydn detected: \"%s\": " "Entry ID: (%d, %d)", @@ -2380,7 +2380,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) "\"%s\" found at line %d of file \"%s\"", slapi_entry_get_dn(entry->ep_entry), fi->line, fi->filename); - idl_free(IDL); + idl_free(&IDL); /* skip this one */ fi->bad = FIFOITEM_BAD; job->skipped++; diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c index 1504fa8..eac265b 100644 --- a/ldap/servers/slapd/back-ldbm/index.c +++ b/ldap/servers/slapd/back-ldbm/index.c @@ -197,7 +197,7 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) goto error; } slapi_ch_free( &(bin->key.data) ); - idl_free(bin->value); + idl_free(&(bin->value)); /* If we're already at allids, store an allids block to prevent needless accumulation of blocks */ if (old_idl && ALLIDS(old_idl)) { bin->value = idl_allids(be); @@ -207,10 +207,10 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) } error: if (old_idl) { - idl_free(old_idl); + idl_free(&old_idl); } if (new_idl && need_to_freed_new_idl) { - idl_free(new_idl); + idl_free(&new_idl); } dblayer_release_index_file( be, a, db ); return ret; @@ -268,7 +268,7 @@ index_buffer_terminate(void *h) for (i = 0; i < handle->buffer_size; i++) { bin = &(handle->bins[i]); if (bin->value) { - idl_free(bin->value); + idl_free(&(bin->value)); bin->value = NULL; } slapi_ch_free(&(bin->key.data)); @@ -1496,7 +1496,7 @@ index_range_read_ext( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "index_range_read lookthrough_limit exceeded\n"); @@ -1525,7 +1525,7 @@ index_range_read_ext( */ if ( slapi_op_abandoned( pb )) { if (NULL != idl) { - idl_free(idl); + idl_free(&idl); idl = NULL; } LDAPDebug0Args(LDAP_DEBUG_TRACE, @@ -1565,7 +1565,7 @@ index_range_read_ext( /* idl tmp only contains one id */ /* append it at the end here; sort idlist at the end */ if (ALLIDS(tmp)) { - idl_free(idl); + idl_free(&idl); idl = tmp; } else { ID id; @@ -1577,7 +1577,7 @@ index_range_read_ext( 1097, *err); } } - idl_free(tmp); + idl_free(&tmp); } if (ALLIDS(idl)) { LDAPDebug0Args(LDAP_DEBUG_TRACE, diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c index 292f4fd..91397d6 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c @@ -1091,7 +1091,7 @@ entryrdn_get_subordinates(backend *be, bail: if (rc && subordinates && *subordinates) { - idl_free(*subordinates); + idl_free(subordinates); } slapi_ch_free_string(&keybuf); slapi_ch_free((void **)&elem); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 78c4148..3cb9437 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -1491,7 +1491,7 @@ common_return: slapi_ch_free((void**)&child_dns); if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL")) slapi_ch_free((void**)&ldap_result_matcheddn); - idl_free(children); + idl_free(&children); slapi_sdn_done(&dn_newdn); slapi_sdn_done(&dn_newrdn); slapi_sdn_done(&dn_parentdn); @@ -2205,7 +2205,7 @@ moddn_get_children(back_txn *ptxn, } } } while (id!=NOID); - idl_free(candidates); + idl_free(&candidates); slapi_sdn_done(&parentsdn); } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index d0563bd..6dff1f5 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -48,6 +48,16 @@ #include "back-ldbm.h" #include "vlv_srch.h" +/* + * Used for ldap_result passed to ldbm_back_search_cleanup. + * If (ldap_result == LDBM_SRCH_DEFAULT_RESULT) || (ldap_result == LDAP_SUCCESS), + * don't call slapi_send_ldap_result. + * Note: mozldap ldap_result codes are all positive; openldap result codes could + * be negative values. OL (-1) is LDAP_SERVER_DOWN. Thus, it's safe to + * borrow the value here. + */ +#define LDBM_SRCH_DEFAULT_RESULT (-1) + /* prototypes */ static int build_candidate_list( Slapi_PBlock *pb, backend *be, struct backentry *e, const char * base, int scope, @@ -162,12 +172,14 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, char* ldap_result_description, int function_result, struct vlv_request *vlv_request_control, - struct backentry *e) + struct backentry *e, + IDList *candidates) { int estimate = 0; /* estimated search result count */ backend *be; ldbm_instance *inst; back_search_result_set *sr = NULL; + int free_candidates = 1; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); inst = (ldbm_instance *) be->be_instance_info; @@ -180,20 +192,27 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, { sort_spec_free(sort_control); } - if(ldap_result>=LDAP_SUCCESS) - { + if ((ldap_result != LDBM_SRCH_DEFAULT_RESULT) && (ldap_result != LDAP_SUCCESS)) { slapi_send_ldap_result( pb, ldap_result, NULL, ldap_result_description, 0, NULL ); } /* code to free the result set if we don't need it */ /* We get it and check to see if the structure was ever used */ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr); if (sr) { - if (function_result) { + if (function_result) { /* failed case */ slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate); slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL); + if (sr->sr_candidates == candidates) { + free_candidates = 0; + } delete_search_result_set(pb, &sr); + } else if (sr->sr_candidates == candidates) { /* succeeded case */ + free_candidates = 0; } } + if (free_candidates) { + idl_free(&candidates); + } if (vlv_request_control) { berval_done(&vlv_request_control->value); @@ -336,7 +355,7 @@ ldbm_back_search( Slapi_PBlock *pb ) back_search_result_set *sr = NULL; /* Fix for bugid #394184, SD, 20 Jul 00 */ - int tmp_err = -1; /* must be lower than LDAP_SUCCESS */ + int tmp_err = LDBM_SRCH_DEFAULT_RESULT; char * tmp_desc = NULL; /* end Fix for defect #394184 */ @@ -401,7 +420,7 @@ ldbm_back_search( Slapi_PBlock *pb ) */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "Sort Control", - SLAPI_FAIL_GENERAL, NULL, NULL); + SLAPI_FAIL_GENERAL, NULL, NULL, candidates); } else { PRUint64 conn_id; int op_id; @@ -436,7 +455,7 @@ ldbm_back_search( Slapi_PBlock *pb ) */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + &vlv_request_control, NULL, candidates); } else { PRUint64 conn_id; int op_id; @@ -470,7 +489,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Client isn't allowed to do this. */ return ldbm_back_search_cleanup(pb, li, sort_control, rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + &vlv_request_control, NULL, candidates); } } /* @@ -485,7 +504,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Can't have a VLV control without a SORT control */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_SORT_CONTROL_MISSING, "VLV Control", - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } } @@ -538,13 +557,13 @@ ldbm_back_search( Slapi_PBlock *pb ) { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNWILLING_TO_PERFORM, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_VIRTUAL_LIST_VIEW_ERROR, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } else @@ -559,7 +578,7 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_make_sort_response_control(pb, LDAP_UNWILLING_TO_PERFORM, NULL); return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else /* vlv and sorting are not critical, so ignore the control */ { @@ -593,7 +612,7 @@ ldbm_back_search( Slapi_PBlock *pb ) { /* error or referral sent by find_entry */ return ldbm_back_search_cleanup(pb, li, sort_control, - -1, NULL, 1, &vlv_request_control, NULL); + LDBM_SRCH_DEFAULT_RESULT, NULL, 1, &vlv_request_control, NULL, candidates); } } @@ -616,6 +635,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* * Build a list of IDs for this entry and scope */ + vlv_response_control.result = LDAP_SUCCESS; if ((NULL != controls) && (sort) && (vlv)) { /* This candidate list is for vlv, no need for sort only. */ switch (vlv_search_build_candidate_list(pb, basesdn, &vlv_rc, @@ -626,12 +646,12 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case VLV_BLD_LIST_FAILED: return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_SUCCESS: /* Log to the access log the particulars of this sort request */ @@ -652,7 +672,7 @@ ldbm_back_search( Slapi_PBlock *pb ) LDAP_OPERATIONS_ERROR, "Sort Response Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -663,9 +683,9 @@ ldbm_back_search( Slapi_PBlock *pb ) if (rc) { /* Error result sent by build_candidate_list */ - return ldbm_back_search_cleanup(pb, li, sort_control, -1, - NULL, rc, - &vlv_request_control, e); + return ldbm_back_search_cleanup(pb, li, sort_control, + LDBM_SRCH_DEFAULT_RESULT, NULL, rc, + &vlv_request_control, e, candidates); } /* * If we're sorting then we must check what administrative @@ -691,23 +711,33 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then apply the * search filter before sorting. */ - if (virtual_list_view && (NULL != candidates)) - { + if (virtual_list_view && candidates) { IDList *idl = NULL; Slapi_Filter *filter = NULL; slapi_pblock_get( pb, SLAPI_SEARCH_FILTER, &filter ); rc = vlv_filter_candidates(be, pb, candidates, basesdn, - scope, filter, &idl, - lookthrough_limit, time_up); - if (rc == 0) { - idl_free(candidates); - candidates= idl; - } - else - { + scope, filter, &idl, + lookthrough_limit, time_up); + switch (rc) { + case LDAP_SUCCESS: /* Everything OK */ + case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ + case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ + vlv_response_control.result = rc; + idl_free(&candidates); + candidates = idl; + break; + case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ + default: return ldbm_back_search_cleanup(pb, li, sort_control, rc, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); + } + if (is_vlv_critical && rc) { + idl_free(&candidates); + candidates = idl_alloc(0); + tmp_err = rc; + tmp_desc = "VLV Response Control"; + goto vlv_bail; } } /* @@ -725,7 +755,7 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } else @@ -759,21 +789,24 @@ ldbm_back_search( Slapi_PBlock *pb ) /* replace the hard coded return value by the appropriate * LDAP error code */ switch (sort_return_value) { - case LDAP_SUCCESS: /* Everything OK */ - vlv_response_control.result= LDAP_SUCCESS; + case LDAP_SUCCESS: + /* + * we don't want to override an error from vlv + * vlv_response_control.result= LDAP_SUCCESS; + */ break; case LDAP_PROTOCOL_ERROR: /* A protocol error */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ case LDAP_OPERATIONS_ERROR: /* Operation error */ case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ - vlv_response_control.result= LDAP_TIMELIMIT_EXCEEDED; - break; case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ - vlv_response_control.result = LDAP_ADMINLIMIT_EXCEEDED; + if (!vlv_response_control.result) { + vlv_response_control.result = sort_return_value; + } break; case LDAP_OTHER: /* Abandoned */ abandoned = 1; /* So that we don't return a result code */ @@ -792,11 +825,10 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Fix for bugid #394184, SD, 05 Jul 00 */ /* we were not actually returning unavailableCriticalExtension; now fixed (hopefully !) */ - if (is_sorting_critical && (0 != sort_return_value)) - { - idl_free(candidates); + if (is_sorting_critical && sort_return_value) { + idl_free(&candidates); candidates = idl_alloc(0); - tmp_err = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + tmp_err = sort_return_value; tmp_desc = "Sort Response Control"; } /* end Fix for bugid #394184 */ @@ -806,9 +838,9 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_return_value, sort_error_type ) ) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -816,10 +848,9 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then the candidate list * must be trimmed down to just the range of entries requested. */ - if (virtual_list_view) - { - if (NULL != candidates && candidates->b_nids>0) - { + if (virtual_list_view) { + if (candidates && (candidates->b_nids > 0) && + !vlv_response_control.result) { IDList *idl= NULL; back_txn txn = {NULL}; slapi_pblock_get(pb, SLAPI_TXN, &txn.back_txn_txn); @@ -828,7 +859,7 @@ ldbm_back_search( Slapi_PBlock *pb ) &vlv_request_control, &idl, &vlv_response_control, &txn); if(vlv_response_control.result==0) { - idl_free(candidates); + idl_free(&candidates); candidates = idl; } else @@ -836,26 +867,23 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } - } - else - { + } else { vlv_response_control.targetPosition = 0; vlv_response_control.contentCount = 0; - vlv_response_control.result = LDAP_SUCCESS; + /* vlv_response_control.result = LDAP_SUCCESS; Don't override */ } } } - if (virtual_list_view) - { - if(LDAP_SUCCESS != - vlv_make_response_control( pb, &vlv_response_control )) - { +vlv_bail: + if (virtual_list_view) { + if (LDAP_SUCCESS != + vlv_make_response_control( pb, &vlv_response_control )) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "VLV Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } /* Log the VLV operation */ vlv_print_access_log(pb,&vlv_request_control,&vlv_response_control); @@ -885,7 +913,7 @@ ldbm_back_search( Slapi_PBlock *pb ) PR_Unlock(inst->inst_config_mutex); if (ri) { - idl_free(candidates); + idl_free(&candidates); candidates = idl_alloc(0); tmp_err = LDAP_UNWILLING_TO_PERFORM; tmp_desc = "Search is not indexed"; @@ -948,10 +976,10 @@ ldbm_back_search( Slapi_PBlock *pb ) } /* Fix for bugid #394184, SD, 05 Jul 00 */ - /* tmp_err == -1: no error */ + /* tmp_err == LDBM_SRCH_DEFAULT_RESULT: no error */ return ldbm_back_search_cleanup(pb, li, sort_control, tmp_err, tmp_desc, - (tmp_err == -1 ? 0 : -1), - &vlv_request_control, NULL); + (tmp_err == LDBM_SRCH_DEFAULT_RESULT ? 0 : LDBM_SRCH_DEFAULT_RESULT), + &vlv_request_control, NULL, candidates); /* end Fix for bugid #394184 */ } @@ -1212,14 +1240,14 @@ subtree_candidates( e->ep_id, &descendants, &txn, 0); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } else if (!has_tombstone_filter) { *err = ldbm_ancestorid_read_ext(be, &txn, e->ep_id, &descendants, allidslimit); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } /* else == has_tombstone_filter: do nothing */ } @@ -1847,7 +1875,7 @@ delete_search_result_set( Slapi_PBlock *pb, back_search_result_set **sr ) } if ( NULL != (*sr)->sr_candidates ) { - idl_free( (*sr)->sr_candidates ); + idl_free( &((*sr)->sr_candidates) ); } rc = slapi_filter_apply((*sr)->sr_norm_filter, ldbm_search_free_compiled_filter, NULL, &filt_errs); diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c index d803f5d..29a9d48 100644 --- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c +++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c @@ -292,7 +292,7 @@ add_op_attrs(Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *ep, if ( (idl = index_read( be, LDBM_ENTRYDN_STR, indextype_EQUALITY, &bv, NULL, &err )) != NULL ) { pid = idl_firstid( idl ); - idl_free( idl ); + idl_free( &idl ); } else { /* empty idl */ if ( 0 != err && DB_NOTFOUND != err ) { @@ -592,7 +592,7 @@ int update_subordinatecounts(backend *be, import_subcount_stuff *mothers, /* We get the count from the IDL */ sub_count = idl->b_nids; } - idl_free(idl); + idl_free(&idl); } /* Did we get the count ? */ if (found_count) { @@ -891,8 +891,7 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) continue; } id = idl_firstid(idl); - idl_free(idl); - idl = NULL; + idl_free(&idl); } /* @@ -930,8 +929,8 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) idltotal = idl; } else if (idl) { idltmp = idl_union(be, idltotal, idl); - idl_free(idltotal); - idl_free(idl); + idl_free(&idltotal); + idl_free(&idl); idltotal = idltmp; } } /* for (i = 0; include[i]; i++) */ @@ -1333,8 +1332,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) /* allids list is no help at all -- revert to trawling * the whole list. */ ok_index = 0; - idl_free(idl); - idl = NULL; + idl_free(&idl); } idindex = 0; } @@ -1590,7 +1588,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) } bye: if (idl) { - idl_free(idl); + idl_free(&idl); } if (dbc) { dbc->c_close(dbc); @@ -1621,7 +1619,7 @@ bye: } ldbm_back_free_incl_excl(include_suffix, exclude_suffix); - idl_free(eargs.pre_exported_idl); + idl_free(&(eargs.pre_exported_idl)); return( return_value ); } @@ -1935,8 +1933,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) } } else if (ALLIDS(idl)) { /* that's no help. */ - idl_free(idl); - idl = NULL; + idl_free(&idl); } } @@ -2445,7 +2442,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) err_out: backentry_free( &ep ); /* if ep or *ep is NULL, it does nothing */ if (idl) { - idl_free(idl); + idl_free(&idl); } else { dbc->c_close(dbc); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index df3b6f3..2f35702 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -238,7 +238,7 @@ struct backentry * id2entry( backend *be, ID id, back_txn *txn, int *err ); * idl.c */ IDList * idl_alloc( NIDS nids ); -void idl_free( IDList *idl ); +void idl_free( IDList **idl ); NIDS idl_length(IDList *idl); int idl_is_allids(IDList *idl); int idl_append(IDList *idl, ID id); diff --git a/ldap/servers/slapd/back-ldbm/seq.c b/ldap/servers/slapd/back-ldbm/seq.c index 10484fd..e388214 100644 --- a/ldap/servers/slapd/back-ldbm/seq.c +++ b/ldap/servers/slapd/back-ldbm/seq.c @@ -242,7 +242,7 @@ ldbm_back_seq( Slapi_PBlock *pb ) key.flags = 0; for (retry_count = 0; retry_count < IDL_FETCH_RETRY_COUNT; retry_count++) { err = NEW_IDL_DEFAULT; - idl_free(idl); + idl_free(&idl); idl = idl_fetch( be, db, &key, txn.back_txn_txn, ai, &err ); if(err == DB_LOCK_DEADLOCK) { ldbm_nasty("ldbm_back_seq deadlock retry", 1600, err); @@ -297,7 +297,7 @@ ldbm_back_seq( Slapi_PBlock *pb ) } CACHE_RETURN( &inst->inst_cache, &e ); } - idl_free( idl ); + idl_free( &idl ); } dblayer_release_index_file( be, ai, db ); diff --git a/ldap/servers/slapd/back-ldbm/vlv.c b/ldap/servers/slapd/back-ldbm/vlv.c index 5e5d0a5..fcec064 100644 --- a/ldap/servers/slapd/back-ldbm/vlv.c +++ b/ldap/servers/slapd/back-ldbm/vlv.c @@ -1112,7 +1112,7 @@ int vlv_build_idl(PRUint32 start, PRUint32 stop, DB *db, DBC *dbc, goto done; error: - if (idl) idl_free(idl); + if (idl) idl_free(&idl); done: return rc;

1 0

Branch '389-ds-base-1.3.2' - ldap/servers
by Noriko Hosoi 15 May '14

15 May '14

ldap/servers/slapd/back-ldbm/ancestorid.c | 28 ++-- ldap/servers/slapd/back-ldbm/filterindex.c | 34 ++--- ldap/servers/slapd/back-ldbm/idl.c | 92 ++++++------- ldap/servers/slapd/back-ldbm/idl_common.c | 12 - ldap/servers/slapd/back-ldbm/idl_new.c | 20 +-- ldap/servers/slapd/back-ldbm/import-merge.c | 6 ldap/servers/slapd/back-ldbm/import-threads.c | 4 ldap/servers/slapd/back-ldbm/index.c | 16 +- ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 2 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 4 ldap/servers/slapd/back-ldbm/ldbm_search.c | 166 ++++++++++++++----------- ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 23 +-- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2 ldap/servers/slapd/back-ldbm/seq.c | 4 ldap/servers/slapd/back-ldbm/vlv.c | 2 15 files changed, 220 insertions(+), 195 deletions(-) New commits: commit e9f86dab95fd600b414cf1c4cf6f7c733348b758 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Thu May 15 14:43:28 2014 -0700 Ticket #47780 - Some VLV search request causes memory leaks Fix description: . Modified idl_free interface as follows so that passed idl is cleared with NULL once the IDList is successfully freed. -idl_free(IDList *idl) +idl_free(IDList **idl) This change is used to clean up search candidates when ldbm_back_ search_cleanup (ldbm_search.c) is called as an error return. The cleanup function frees the search candidates when it's not NULL and it's not assigned to sr_candidates field in the search result. This fixes a memory leak when VLV/Sort op fails. . ldbm_back_search_cleanup (ldbm_search.c) calls slapi_send_ldap_result if an ldap error is passed to the function. The logic used to be "if (ldap_result>=LDAP_SUCCESS)", which is based upon that mozldap return codes are all positive. Supporting openldap library, there is a chance to get a negative return code (e.g. LDAP_PARAM_ERROR == -9). This patch supports the negative return codes, as well. . In ldbm_back_search (ldbm_search.c) vlv_filter_candidates could ruturn errors such as and LDAP_TIMELIMIT_EXCEEDED, LDAP_ADMINLIMIT_ EXCEEDED. The search results are supposed to be returned to the client with the error code if the control is not critical. The code is added. . The VLV operation stores the result in vlv_response_control.result in ldbm_back_search (ldbm_search.c), which occurs at 3 places, vlv_ filter_candidates, sort_candidates and vlv_trim_candidates_txn. The return code from the latter calls used to override the former return code. This patch fixes it to respect the former return code. https://fedorahosted.org/389/ticket/47780 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) (cherry picked from commit 1118cc9b61a60c704250080d6af8785e4f8d28af) diff --git a/ldap/servers/slapd/back-ldbm/ancestorid.c b/ldap/servers/slapd/back-ldbm/ancestorid.c index 8722b68..54b08ad 100644 --- a/ldap/servers/slapd/back-ldbm/ancestorid.c +++ b/ldap/servers/slapd/back-ldbm/ancestorid.c @@ -136,7 +136,7 @@ static int ldbm_get_nonleaf_ids(backend *be, DB_TXN *txn, IDList **idl) LDAPDebug(LDAP_DEBUG_TRACE, "found %lu nodes for ancestorid\n", (u_long)IDL_NIDS(nodes), 0, 0); } else { - idl_free(nodes); + idl_free(&nodes); *idl = NULL; } @@ -264,7 +264,7 @@ static int ldbm_ancestorid_default_create_index(backend *be) /* Insert into ancestorid for this node */ if (id2idl_hash_lookup(ht, &id, &ididl)) { descendants = idl_union_allids(be, ai_aid, ididl->idl, children); - idl_free(children); + idl_free(&children); if (id2idl_hash_remove(ht, &id) == 0) { LDAPDebug(LDAP_DEBUG_ANY, "ancestorid hash_remove failed\n", 0,0,0); } else { @@ -279,21 +279,21 @@ static int ldbm_ancestorid_default_create_index(backend *be) /* Get parentid for this entry */ ret = ldbm_parentid(be, txn, id, &parentid); if (ret != 0) { - idl_free(descendants); + idl_free(&descendants); break; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(descendants); + idl_free(&descendants); continue; } /* Insert into ancestorid for this node's parent */ if (id2idl_hash_lookup(ht, &parentid, &ididl)) { IDList *idl = idl_union_allids(be, ai_aid, ididl->idl, descendants); - idl_free(descendants); - idl_free(ididl->idl); + idl_free(&descendants); + idl_free(&(ididl->idl)); ididl->idl = idl; } else { ididl = (id2idl*)slapi_ch_calloc(1,sizeof(id2idl)); @@ -324,7 +324,7 @@ static int ldbm_ancestorid_default_create_index(backend *be) id2idl_hash_destroy(ht); /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -456,7 +456,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) /* Insert into ancestorid for this node */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); break; } @@ -467,13 +467,13 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) slapi_log_error(SLAPI_LOG_FATAL, sourcefile, "Error: ldbm_parentid on node index [" ID_FMT "] of [" ID_FMT "]\n", nids, nodes->b_nids); - idl_free(children); + idl_free(&children); goto out; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(children); + idl_free(&children); break; } @@ -485,7 +485,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) /* Insert into ancestorid for this node's parent */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); goto out; } id = parentid; @@ -504,7 +504,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be) } /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -583,7 +583,7 @@ static int ldbm_parentid(backend *be, DB_TXN *txn, ID id, ID *ppid) static void id2idl_free(id2idl **ididl) { - idl_free((*ididl)->idl); + idl_free(&((*ididl)->idl)); slapi_ch_free((void**)ididl); } @@ -785,7 +785,7 @@ static int ldbm_ancestorid_index_update( break; } node_id = idl_firstid(idl); - idl_free(idl); + idl_free(&idl); } /* Update ancestorid for the base entry */ diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c index 9ad2ab4..c8c40c4 100644 --- a/ldap/servers/slapd/back-ldbm/filterindex.c +++ b/ldap/servers/slapd/back-ldbm/filterindex.c @@ -399,7 +399,7 @@ presence_candidates( LDAPDebug(LDAP_DEBUG_TRACE, "fallback to eq index as pres index gave allids\n", 0, 0, 0); - idl_free(idl); + idl_free(&idl); idl = index_range_read_ext(pb, be, type, indextype_EQUALITY, SLAPI_OP_GREATER_OR_EQUAL, NULL, NULL, 0, &txn, err, allidslimit); @@ -481,7 +481,7 @@ extensible_candidates( else if (keys == NULL || keys[0] == NULL) { /* no keys */ - idl_free (idl); + idl_free (&idl); idl = idl_allids (be); } else @@ -516,8 +516,8 @@ extensible_candidates( else { IDList* tmp = idl_intersection (be, idl2, idl3); - idl_free (idl2); - idl_free (idl3); + idl_free (&idl2); + idl_free (&idl3); idl2 = tmp; } if (idl2 == NULL) break; /* look no further */ @@ -529,8 +529,8 @@ extensible_candidates( else if (idl2 != NULL) { IDList* tmp = idl_union (be, idl, idl2); - idl_free (idl); - idl_free (idl2); + idl_free (&idl); + idl_free (&idl2); idl = tmp; } } @@ -828,7 +828,7 @@ list_candidates( { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -838,7 +838,7 @@ list_candidates( == NULL && ftype == LDAP_FILTER_AND ) { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -862,15 +862,15 @@ list_candidates( int notin_result = 0; notin_result = idl_notin( be, idl, tmp, &new_idl ); if (notin_result) { - idl_free(idl); + idl_free(&idl); idl = new_idl; } } } else { idl = idl_intersection(be, idl, tmp); - idl_free( tmp2 ); + idl_free( &tmp2 ); } - idl_free( tmp ); + idl_free( &tmp ); /* stop if the list has gotten too small */ if ((idl == NULL) || (idl_length(idl) <= FILTER_TEST_THRESHOLD)) @@ -880,8 +880,8 @@ list_candidates( slapi_pblock_get( pb, SLAPI_OPERATION, &operation ); idl = idl_union( be, idl, tmp ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); /* stop if we're already committed to an exhaustive * search. :( */ @@ -890,7 +890,7 @@ list_candidates( if (op_is_pagedresults(operation)) { int nids = IDL_NIDS(idl); if ( allidslimit > 0 && nids > allidslimit ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } } @@ -1013,7 +1013,7 @@ keys2idl( } #endif if ( idl2 == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl = NULL; break; } @@ -1025,8 +1025,8 @@ keys2idl( tmp = idl; idl = idl_intersection(be, idl, idl2); - idl_free( idl2 ); - idl_free( tmp ); + idl_free( &idl2 ); + idl_free( &tmp ); if ( idl == NULL ) { break; } diff --git a/ldap/servers/slapd/back-ldbm/idl.c b/ldap/servers/slapd/back-ldbm/idl.c index a0bc139..6a7c7b0 100644 --- a/ldap/servers/slapd/back-ldbm/idl.c +++ b/ldap/servers/slapd/back-ldbm/idl.c @@ -284,12 +284,12 @@ idl_old_fetch( if ( ! INDIRECT_BLOCK( idl ) ) { /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); } - idl_free( idl ); + idl_free( &idl ); /* Taking a transaction is expensive; so we try and optimize for the common case by not taking one above. If we have a indirect block; we need to take a transaction and re-read @@ -311,7 +311,7 @@ idl_old_fetch( dblayer_read_txn_commit(be, &s_txn); /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); @@ -371,7 +371,7 @@ idl_old_fetch( dblayer_read_txn_commit(be, &s_txn); tmp[i] = NULL; slapi_ch_free((void**)&kstr ); - idl_free( idl ); + idl_free( &idl ); /* allocate space for the big block */ idl = idl_alloc( nids ); @@ -388,7 +388,7 @@ idl_old_fetch( tmp[i]->b_nids * sizeof(ID) ); nids += tmp[i]->b_nids; - idl_free( tmp[i] ); + idl_free( &tmp[i] ); } slapi_ch_free((void**)&tmp ); @@ -633,7 +633,7 @@ idl_old_insert_key( rc, (msg = dblayer_strerror( rc )) ? msg : "", 0 ); } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -659,10 +659,10 @@ idl_old_insert_key( case 3: /* id not inserted - block must be split */ /* check threshold for marking this an all-id block */ if ( a->ai_idl->idl_maxindirect < 2 ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK) @@ -677,7 +677,7 @@ idl_old_insert_key( } idl_split_block( idl, id, &tmp, &tmp2 ); - idl_free( idl ); + idl_free( &idl ); /* create the header indirect block */ idl = idl_alloc( 3 ); @@ -690,9 +690,9 @@ idl_old_insert_key( /* store it */ rc = idl_store( be, db, key, idl, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 3 BAD %d %s\n", @@ -716,9 +716,9 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 4 BAD %d %s\n", @@ -730,12 +730,12 @@ idl_old_insert_key( "idl_insert_key", "split", key, id); slapi_ch_free((void**)&kstr ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) { @@ -763,7 +763,7 @@ idl_old_insert_key( (u_long)id, key.dptr, i); #endif idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); return( 0 ); } if ( i != 0 ) { @@ -787,7 +787,7 @@ idl_old_insert_key( LDAPDebug( LDAP_DEBUG_ANY, "nonexistent continuation block (%s)\n", k2.dptr, 0, 0 ); idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); slapi_ch_free((void**)&kstr ); return( -1 ); } @@ -915,9 +915,9 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( tmp2 ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); @@ -945,7 +945,7 @@ idl_old_insert_key( * which is not correct. */ rc = 0; - idl_free( tmp2 ); + idl_free( &tmp2 ); break; } if ( rc != 0 ) { @@ -990,7 +990,7 @@ idl_old_insert_key( /* store allid block in place of header block */ if ( 0 == rc ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); if (NULL != disposition) { @@ -1000,14 +1000,14 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( idl ); - idl_free( tmp ); + idl_free( &idl ); + idl_free( &tmp ); idl_unlock_list(a->ai_idl,key); return( rc ); } idl_split_block( tmp, id, &tmp2, &tmp3 ); - idl_free( tmp ); + idl_free( &tmp ); /* create a new updated indirect header block */ tmp = idl_alloc( idl->b_nmax + 1 ); @@ -1025,8 +1025,8 @@ idl_old_insert_key( /* store the header block */ rc = idl_store( be, db, key, tmp, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1037,8 +1037,8 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1049,22 +1049,22 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp3, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } idl_check_indirect (tmp, i, tmp2, tmp3, "idl_insert_key", "indirect split", key, id); - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -1100,7 +1100,7 @@ int idl_old_store_block( /* If so, store an ALLIDS block */ IDList *all = idl_allids(be); ret = idl_store(be,db,key,all,txn); - idl_free(all); + idl_free(&all); } else { /* Then , is it a block which is smaller than the size at which it needs splitting ? */ if (idl->b_nids <= (ID)priv->idl_maxids) { @@ -1159,7 +1159,7 @@ int idl_old_store_block( make_cont_key(&cont_key,key,lead_id); /* Now store the continuation block */ ret = idl_store(be,db,&cont_key,this_cont_block,txn); - idl_free(this_cont_block); + idl_free(&this_cont_block); slapi_ch_free(&(cont_key.data)); if ( ret != 0 && ret != DB_LOCK_DEADLOCK ) { @@ -1181,7 +1181,7 @@ int idl_old_store_block( } done: /* Free master block */ - idl_free(master_block); + idl_free(&master_block); return ret; } @@ -1442,7 +1442,7 @@ idl_old_delete_key( break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) %d (not indirect)\n", key->dptr, (u_long)id, rc ); @@ -1463,7 +1463,7 @@ idl_old_delete_key( } /* id smaller than smallest id - not there */ if ( i == 0 && id < idl->b_ids[i] ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) -666 (id not found)\n", key->dptr, (u_long)id, 0 ); @@ -1476,7 +1476,7 @@ idl_old_delete_key( /* get the block to delete from */ make_cont_key( &contkey, key, idl->b_ids[i] ); if ( (didl = idl_fetch_one( li, db, &contkey, txn, &rc )) == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != DB_LOCK_DEADLOCK ) { @@ -1563,8 +1563,8 @@ idl_old_delete_key( rc = 0; break; } - idl_free( idl ); - idl_free( didl ); + idl_free( &idl ); + idl_free( &didl ); slapi_ch_free( (void **)&(contkey.dptr) ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) diff --git a/ldap/servers/slapd/back-ldbm/idl_common.c b/ldap/servers/slapd/back-ldbm/idl_common.c index b6d2e68..97b2e36 100644 --- a/ldap/servers/slapd/back-ldbm/idl_common.c +++ b/ldap/servers/slapd/back-ldbm/idl_common.c @@ -94,13 +94,13 @@ idl_allids( backend *be ) } void -idl_free( IDList *idl ) /* JCM - pass in ** */ +idl_free( IDList **idl ) { - if ( idl == NULL ) { + if ((NULL == idl) || (NULL == *idl)) { return; } - slapi_ch_free((void**)&idl ); + slapi_ch_free((void**)idl); } @@ -163,7 +163,7 @@ idl_append_extend(IDList **orig_idl, ID id) /* copy over the existing contents */ idl_new->b_nids = idl->b_nids; memcpy(idl_new->b_ids, idl->b_ids, sizeof(ID) * idl->b_nids); - idl_free(idl); + idl_free(&idl); idl = idl_new; } @@ -255,7 +255,7 @@ idl_intersection( } if ( ni == 0 ) { - idl_free( n ); + idl_free( &n ); return( NULL ); } n->b_nids = ni; @@ -367,7 +367,7 @@ idl_notin( } if ( ni == n->b_nmax ) { - idl_free( n ); + idl_free( &n ); *new_result = idl_allids( be ); } else { n->b_nids = ni; diff --git a/ldap/servers/slapd/back-ldbm/idl_new.c b/ldap/servers/slapd/back-ldbm/idl_new.c index f0410f9..120153e 100644 --- a/ldap/servers/slapd/back-ldbm/idl_new.c +++ b/ldap/servers/slapd/back-ldbm/idl_new.c @@ -285,7 +285,7 @@ idl_new_fetch( idl_rc = idl_append_extend(&idl, id); if (idl_rc) { LDAPDebug(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc, 0, 0); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -323,7 +323,7 @@ idl_new_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -339,7 +339,7 @@ idl_new_fetch( #endif if (ret != DB_NOTFOUND) { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -348,7 +348,7 @@ idl_new_fetch( /* check for allids value */ if (idl != NULL && idl->b_nids == 1 && idl->b_ids[0] == ALLID) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", key.data, 0, 0); @@ -501,7 +501,7 @@ idl_new_range_fetch( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "idl_new_range_fetch - lookthrough_limit exceeded\n"); @@ -557,7 +557,7 @@ idl_new_range_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -618,7 +618,7 @@ idl_new_range_fetch( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "idl_new_range_fetch - lookthrough_limit exceeded\n"); @@ -669,7 +669,7 @@ idl_new_range_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -688,7 +688,7 @@ idl_new_range_fetch( if (ret == DB_NOTFOUND) { ret = 0; /* normal case */ } else { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -696,7 +696,7 @@ idl_new_range_fetch( /* check for allids value */ if (idl && (idl->b_nids == 1) && (idl->b_ids[0] == ALLID)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug1Arg(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", cur_key.data); diff --git a/ldap/servers/slapd/back-ldbm/import-merge.c b/ldap/servers/slapd/back-ldbm/import-merge.c index 223f34f..a728165 100644 --- a/ldap/servers/slapd/back-ldbm/import-merge.c +++ b/ldap/servers/slapd/back-ldbm/import-merge.c @@ -169,8 +169,8 @@ static int import_merge_insert_input_queue(backend *be, import_merge_queue_entry IDList *new_idl = idl_union(be, current_entry->thang.payload.idl, idl); - idl_free(current_entry->thang.payload.idl); - idl_free(idl); + idl_free(&(current_entry->thang.payload.idl)); + idl_free(&idl); current_entry->thang.payload.idl = new_idl; /* Add this file id into the entry's referenced list */ (current_entry->file_referenced_list)[fileno] = 1; @@ -567,7 +567,7 @@ static int import_merge_one_file(ImportWorkerInfo *worker, int passes, ret = idl_store_block(be, output_file, &key, thang.payload.idl, NULL, worker->index_info->ai); /* Free the key we got back from the queue */ - idl_free(thang.payload.idl); + idl_free(&(thang.payload.idl)); thang.payload.idl = NULL; } slapi_ch_free(&(key.data)); diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c index 96734ac..f763229 100644 --- a/ldap/servers/slapd/back-ldbm/import-threads.c +++ b/ldap/servers/slapd/back-ldbm/import-threads.c @@ -2354,7 +2354,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) */ if (IDL) { ID id = idl_firstid(IDL); /* entrydn is a single attr */ - idl_free(IDL); + idl_free(&IDL); if (id != entry->ep_id) { /* case (2) */ import_log_notice(job, "Duplicated entrydn detected: \"%s\": " "Entry ID: (%d, %d)", @@ -2380,7 +2380,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) "\"%s\" found at line %d of file \"%s\"", slapi_entry_get_dn(entry->ep_entry), fi->line, fi->filename); - idl_free(IDL); + idl_free(&IDL); /* skip this one */ fi->bad = FIFOITEM_BAD; job->skipped++; diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c index 5610b49..ffeedef 100644 --- a/ldap/servers/slapd/back-ldbm/index.c +++ b/ldap/servers/slapd/back-ldbm/index.c @@ -197,7 +197,7 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) goto error; } slapi_ch_free( &(bin->key.data) ); - idl_free(bin->value); + idl_free(&(bin->value)); /* If we're already at allids, store an allids block to prevent needless accumulation of blocks */ if (old_idl && ALLIDS(old_idl)) { bin->value = idl_allids(be); @@ -207,10 +207,10 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) } error: if (old_idl) { - idl_free(old_idl); + idl_free(&old_idl); } if (new_idl && need_to_freed_new_idl) { - idl_free(new_idl); + idl_free(&new_idl); } dblayer_release_index_file( be, a, db ); return ret; @@ -268,7 +268,7 @@ index_buffer_terminate(void *h) for (i = 0; i < handle->buffer_size; i++) { bin = &(handle->bins[i]); if (bin->value) { - idl_free(bin->value); + idl_free(&(bin->value)); bin->value = NULL; } slapi_ch_free(&(bin->key.data)); @@ -1505,7 +1505,7 @@ index_range_read_ext( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "index_range_read lookthrough_limit exceeded\n"); @@ -1534,7 +1534,7 @@ index_range_read_ext( */ if ( slapi_op_abandoned( pb )) { if (NULL != idl) { - idl_free(idl); + idl_free(&idl); idl = NULL; } LDAPDebug0Args(LDAP_DEBUG_TRACE, @@ -1574,7 +1574,7 @@ index_range_read_ext( /* idl tmp only contains one id */ /* append it at the end here; sort idlist at the end */ if (ALLIDS(tmp)) { - idl_free(idl); + idl_free(&idl); idl = tmp; } else { ID id; @@ -1586,7 +1586,7 @@ index_range_read_ext( 1097, *err); } } - idl_free(tmp); + idl_free(&tmp); } if (ALLIDS(idl)) { LDAPDebug0Args(LDAP_DEBUG_TRACE, diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c index 292f4fd..91397d6 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c @@ -1091,7 +1091,7 @@ entryrdn_get_subordinates(backend *be, bail: if (rc && subordinates && *subordinates) { - idl_free(*subordinates); + idl_free(subordinates); } slapi_ch_free_string(&keybuf); slapi_ch_free((void **)&elem); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 02374cd..71056bb 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -1491,7 +1491,7 @@ common_return: slapi_ch_free((void**)&child_dns); if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL")) slapi_ch_free((void**)&ldap_result_matcheddn); - idl_free(children); + idl_free(&children); slapi_sdn_done(&dn_newdn); slapi_sdn_done(&dn_newrdn); slapi_sdn_done(&dn_parentdn); @@ -2205,7 +2205,7 @@ moddn_get_children(back_txn *ptxn, } } } while (id!=NOID); - idl_free(candidates); + idl_free(&candidates); slapi_sdn_done(&parentsdn); } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index d0563bd..6dff1f5 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -48,6 +48,16 @@ #include "back-ldbm.h" #include "vlv_srch.h" +/* + * Used for ldap_result passed to ldbm_back_search_cleanup. + * If (ldap_result == LDBM_SRCH_DEFAULT_RESULT) || (ldap_result == LDAP_SUCCESS), + * don't call slapi_send_ldap_result. + * Note: mozldap ldap_result codes are all positive; openldap result codes could + * be negative values. OL (-1) is LDAP_SERVER_DOWN. Thus, it's safe to + * borrow the value here. + */ +#define LDBM_SRCH_DEFAULT_RESULT (-1) + /* prototypes */ static int build_candidate_list( Slapi_PBlock *pb, backend *be, struct backentry *e, const char * base, int scope, @@ -162,12 +172,14 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, char* ldap_result_description, int function_result, struct vlv_request *vlv_request_control, - struct backentry *e) + struct backentry *e, + IDList *candidates) { int estimate = 0; /* estimated search result count */ backend *be; ldbm_instance *inst; back_search_result_set *sr = NULL; + int free_candidates = 1; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); inst = (ldbm_instance *) be->be_instance_info; @@ -180,20 +192,27 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, { sort_spec_free(sort_control); } - if(ldap_result>=LDAP_SUCCESS) - { + if ((ldap_result != LDBM_SRCH_DEFAULT_RESULT) && (ldap_result != LDAP_SUCCESS)) { slapi_send_ldap_result( pb, ldap_result, NULL, ldap_result_description, 0, NULL ); } /* code to free the result set if we don't need it */ /* We get it and check to see if the structure was ever used */ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr); if (sr) { - if (function_result) { + if (function_result) { /* failed case */ slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate); slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL); + if (sr->sr_candidates == candidates) { + free_candidates = 0; + } delete_search_result_set(pb, &sr); + } else if (sr->sr_candidates == candidates) { /* succeeded case */ + free_candidates = 0; } } + if (free_candidates) { + idl_free(&candidates); + } if (vlv_request_control) { berval_done(&vlv_request_control->value); @@ -336,7 +355,7 @@ ldbm_back_search( Slapi_PBlock *pb ) back_search_result_set *sr = NULL; /* Fix for bugid #394184, SD, 20 Jul 00 */ - int tmp_err = -1; /* must be lower than LDAP_SUCCESS */ + int tmp_err = LDBM_SRCH_DEFAULT_RESULT; char * tmp_desc = NULL; /* end Fix for defect #394184 */ @@ -401,7 +420,7 @@ ldbm_back_search( Slapi_PBlock *pb ) */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "Sort Control", - SLAPI_FAIL_GENERAL, NULL, NULL); + SLAPI_FAIL_GENERAL, NULL, NULL, candidates); } else { PRUint64 conn_id; int op_id; @@ -436,7 +455,7 @@ ldbm_back_search( Slapi_PBlock *pb ) */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + &vlv_request_control, NULL, candidates); } else { PRUint64 conn_id; int op_id; @@ -470,7 +489,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Client isn't allowed to do this. */ return ldbm_back_search_cleanup(pb, li, sort_control, rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + &vlv_request_control, NULL, candidates); } } /* @@ -485,7 +504,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Can't have a VLV control without a SORT control */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_SORT_CONTROL_MISSING, "VLV Control", - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } } @@ -538,13 +557,13 @@ ldbm_back_search( Slapi_PBlock *pb ) { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNWILLING_TO_PERFORM, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_VIRTUAL_LIST_VIEW_ERROR, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } else @@ -559,7 +578,7 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_make_sort_response_control(pb, LDAP_UNWILLING_TO_PERFORM, NULL); return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else /* vlv and sorting are not critical, so ignore the control */ { @@ -593,7 +612,7 @@ ldbm_back_search( Slapi_PBlock *pb ) { /* error or referral sent by find_entry */ return ldbm_back_search_cleanup(pb, li, sort_control, - -1, NULL, 1, &vlv_request_control, NULL); + LDBM_SRCH_DEFAULT_RESULT, NULL, 1, &vlv_request_control, NULL, candidates); } } @@ -616,6 +635,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* * Build a list of IDs for this entry and scope */ + vlv_response_control.result = LDAP_SUCCESS; if ((NULL != controls) && (sort) && (vlv)) { /* This candidate list is for vlv, no need for sort only. */ switch (vlv_search_build_candidate_list(pb, basesdn, &vlv_rc, @@ -626,12 +646,12 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case VLV_BLD_LIST_FAILED: return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_SUCCESS: /* Log to the access log the particulars of this sort request */ @@ -652,7 +672,7 @@ ldbm_back_search( Slapi_PBlock *pb ) LDAP_OPERATIONS_ERROR, "Sort Response Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -663,9 +683,9 @@ ldbm_back_search( Slapi_PBlock *pb ) if (rc) { /* Error result sent by build_candidate_list */ - return ldbm_back_search_cleanup(pb, li, sort_control, -1, - NULL, rc, - &vlv_request_control, e); + return ldbm_back_search_cleanup(pb, li, sort_control, + LDBM_SRCH_DEFAULT_RESULT, NULL, rc, + &vlv_request_control, e, candidates); } /* * If we're sorting then we must check what administrative @@ -691,23 +711,33 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then apply the * search filter before sorting. */ - if (virtual_list_view && (NULL != candidates)) - { + if (virtual_list_view && candidates) { IDList *idl = NULL; Slapi_Filter *filter = NULL; slapi_pblock_get( pb, SLAPI_SEARCH_FILTER, &filter ); rc = vlv_filter_candidates(be, pb, candidates, basesdn, - scope, filter, &idl, - lookthrough_limit, time_up); - if (rc == 0) { - idl_free(candidates); - candidates= idl; - } - else - { + scope, filter, &idl, + lookthrough_limit, time_up); + switch (rc) { + case LDAP_SUCCESS: /* Everything OK */ + case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ + case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ + vlv_response_control.result = rc; + idl_free(&candidates); + candidates = idl; + break; + case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ + default: return ldbm_back_search_cleanup(pb, li, sort_control, rc, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); + } + if (is_vlv_critical && rc) { + idl_free(&candidates); + candidates = idl_alloc(0); + tmp_err = rc; + tmp_desc = "VLV Response Control"; + goto vlv_bail; } } /* @@ -725,7 +755,7 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } else @@ -759,21 +789,24 @@ ldbm_back_search( Slapi_PBlock *pb ) /* replace the hard coded return value by the appropriate * LDAP error code */ switch (sort_return_value) { - case LDAP_SUCCESS: /* Everything OK */ - vlv_response_control.result= LDAP_SUCCESS; + case LDAP_SUCCESS: + /* + * we don't want to override an error from vlv + * vlv_response_control.result= LDAP_SUCCESS; + */ break; case LDAP_PROTOCOL_ERROR: /* A protocol error */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ case LDAP_OPERATIONS_ERROR: /* Operation error */ case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ - vlv_response_control.result= LDAP_TIMELIMIT_EXCEEDED; - break; case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ - vlv_response_control.result = LDAP_ADMINLIMIT_EXCEEDED; + if (!vlv_response_control.result) { + vlv_response_control.result = sort_return_value; + } break; case LDAP_OTHER: /* Abandoned */ abandoned = 1; /* So that we don't return a result code */ @@ -792,11 +825,10 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Fix for bugid #394184, SD, 05 Jul 00 */ /* we were not actually returning unavailableCriticalExtension; now fixed (hopefully !) */ - if (is_sorting_critical && (0 != sort_return_value)) - { - idl_free(candidates); + if (is_sorting_critical && sort_return_value) { + idl_free(&candidates); candidates = idl_alloc(0); - tmp_err = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + tmp_err = sort_return_value; tmp_desc = "Sort Response Control"; } /* end Fix for bugid #394184 */ @@ -806,9 +838,9 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_return_value, sort_error_type ) ) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -816,10 +848,9 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then the candidate list * must be trimmed down to just the range of entries requested. */ - if (virtual_list_view) - { - if (NULL != candidates && candidates->b_nids>0) - { + if (virtual_list_view) { + if (candidates && (candidates->b_nids > 0) && + !vlv_response_control.result) { IDList *idl= NULL; back_txn txn = {NULL}; slapi_pblock_get(pb, SLAPI_TXN, &txn.back_txn_txn); @@ -828,7 +859,7 @@ ldbm_back_search( Slapi_PBlock *pb ) &vlv_request_control, &idl, &vlv_response_control, &txn); if(vlv_response_control.result==0) { - idl_free(candidates); + idl_free(&candidates); candidates = idl; } else @@ -836,26 +867,23 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } - } - else - { + } else { vlv_response_control.targetPosition = 0; vlv_response_control.contentCount = 0; - vlv_response_control.result = LDAP_SUCCESS; + /* vlv_response_control.result = LDAP_SUCCESS; Don't override */ } } } - if (virtual_list_view) - { - if(LDAP_SUCCESS != - vlv_make_response_control( pb, &vlv_response_control )) - { +vlv_bail: + if (virtual_list_view) { + if (LDAP_SUCCESS != + vlv_make_response_control( pb, &vlv_response_control )) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "VLV Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } /* Log the VLV operation */ vlv_print_access_log(pb,&vlv_request_control,&vlv_response_control); @@ -885,7 +913,7 @@ ldbm_back_search( Slapi_PBlock *pb ) PR_Unlock(inst->inst_config_mutex); if (ri) { - idl_free(candidates); + idl_free(&candidates); candidates = idl_alloc(0); tmp_err = LDAP_UNWILLING_TO_PERFORM; tmp_desc = "Search is not indexed"; @@ -948,10 +976,10 @@ ldbm_back_search( Slapi_PBlock *pb ) } /* Fix for bugid #394184, SD, 05 Jul 00 */ - /* tmp_err == -1: no error */ + /* tmp_err == LDBM_SRCH_DEFAULT_RESULT: no error */ return ldbm_back_search_cleanup(pb, li, sort_control, tmp_err, tmp_desc, - (tmp_err == -1 ? 0 : -1), - &vlv_request_control, NULL); + (tmp_err == LDBM_SRCH_DEFAULT_RESULT ? 0 : LDBM_SRCH_DEFAULT_RESULT), + &vlv_request_control, NULL, candidates); /* end Fix for bugid #394184 */ } @@ -1212,14 +1240,14 @@ subtree_candidates( e->ep_id, &descendants, &txn, 0); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } else if (!has_tombstone_filter) { *err = ldbm_ancestorid_read_ext(be, &txn, e->ep_id, &descendants, allidslimit); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } /* else == has_tombstone_filter: do nothing */ } @@ -1847,7 +1875,7 @@ delete_search_result_set( Slapi_PBlock *pb, back_search_result_set **sr ) } if ( NULL != (*sr)->sr_candidates ) { - idl_free( (*sr)->sr_candidates ); + idl_free( &((*sr)->sr_candidates) ); } rc = slapi_filter_apply((*sr)->sr_norm_filter, ldbm_search_free_compiled_filter, NULL, &filt_errs); diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c index d803f5d..29a9d48 100644 --- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c +++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c @@ -292,7 +292,7 @@ add_op_attrs(Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *ep, if ( (idl = index_read( be, LDBM_ENTRYDN_STR, indextype_EQUALITY, &bv, NULL, &err )) != NULL ) { pid = idl_firstid( idl ); - idl_free( idl ); + idl_free( &idl ); } else { /* empty idl */ if ( 0 != err && DB_NOTFOUND != err ) { @@ -592,7 +592,7 @@ int update_subordinatecounts(backend *be, import_subcount_stuff *mothers, /* We get the count from the IDL */ sub_count = idl->b_nids; } - idl_free(idl); + idl_free(&idl); } /* Did we get the count ? */ if (found_count) { @@ -891,8 +891,7 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) continue; } id = idl_firstid(idl); - idl_free(idl); - idl = NULL; + idl_free(&idl); } /* @@ -930,8 +929,8 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) idltotal = idl; } else if (idl) { idltmp = idl_union(be, idltotal, idl); - idl_free(idltotal); - idl_free(idl); + idl_free(&idltotal); + idl_free(&idl); idltotal = idltmp; } } /* for (i = 0; include[i]; i++) */ @@ -1333,8 +1332,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) /* allids list is no help at all -- revert to trawling * the whole list. */ ok_index = 0; - idl_free(idl); - idl = NULL; + idl_free(&idl); } idindex = 0; } @@ -1590,7 +1588,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) } bye: if (idl) { - idl_free(idl); + idl_free(&idl); } if (dbc) { dbc->c_close(dbc); @@ -1621,7 +1619,7 @@ bye: } ldbm_back_free_incl_excl(include_suffix, exclude_suffix); - idl_free(eargs.pre_exported_idl); + idl_free(&(eargs.pre_exported_idl)); return( return_value ); } @@ -1935,8 +1933,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) } } else if (ALLIDS(idl)) { /* that's no help. */ - idl_free(idl); - idl = NULL; + idl_free(&idl); } } @@ -2445,7 +2442,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) err_out: backentry_free( &ep ); /* if ep or *ep is NULL, it does nothing */ if (idl) { - idl_free(idl); + idl_free(&idl); } else { dbc->c_close(dbc); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 97636b8..0022bfd 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -242,7 +242,7 @@ struct backentry * id2entry( backend *be, ID id, back_txn *txn, int *err ); * idl.c */ IDList * idl_alloc( NIDS nids ); -void idl_free( IDList *idl ); +void idl_free( IDList **idl ); NIDS idl_length(IDList *idl); int idl_is_allids(IDList *idl); int idl_append(IDList *idl, ID id); diff --git a/ldap/servers/slapd/back-ldbm/seq.c b/ldap/servers/slapd/back-ldbm/seq.c index 10484fd..e388214 100644 --- a/ldap/servers/slapd/back-ldbm/seq.c +++ b/ldap/servers/slapd/back-ldbm/seq.c @@ -242,7 +242,7 @@ ldbm_back_seq( Slapi_PBlock *pb ) key.flags = 0; for (retry_count = 0; retry_count < IDL_FETCH_RETRY_COUNT; retry_count++) { err = NEW_IDL_DEFAULT; - idl_free(idl); + idl_free(&idl); idl = idl_fetch( be, db, &key, txn.back_txn_txn, ai, &err ); if(err == DB_LOCK_DEADLOCK) { ldbm_nasty("ldbm_back_seq deadlock retry", 1600, err); @@ -297,7 +297,7 @@ ldbm_back_seq( Slapi_PBlock *pb ) } CACHE_RETURN( &inst->inst_cache, &e ); } - idl_free( idl ); + idl_free( &idl ); } dblayer_release_index_file( be, ai, db ); diff --git a/ldap/servers/slapd/back-ldbm/vlv.c b/ldap/servers/slapd/back-ldbm/vlv.c index 5e5d0a5..fcec064 100644 --- a/ldap/servers/slapd/back-ldbm/vlv.c +++ b/ldap/servers/slapd/back-ldbm/vlv.c @@ -1112,7 +1112,7 @@ int vlv_build_idl(PRUint32 start, PRUint32 stop, DB *db, DBC *dbc, goto done; error: - if (idl) idl_free(idl); + if (idl) idl_free(&idl); done: return rc;

1 0

ldap/servers
by Noriko Hosoi 15 May '14

15 May '14

ldap/servers/slapd/back-ldbm/ancestorid.c | 28 ++-- ldap/servers/slapd/back-ldbm/filterindex.c | 34 ++--- ldap/servers/slapd/back-ldbm/idl.c | 92 ++++++------- ldap/servers/slapd/back-ldbm/idl_common.c | 12 - ldap/servers/slapd/back-ldbm/idl_new.c | 20 +-- ldap/servers/slapd/back-ldbm/import-merge.c | 6 ldap/servers/slapd/back-ldbm/import-threads.c | 4 ldap/servers/slapd/back-ldbm/index.c | 16 +- ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 2 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 4 ldap/servers/slapd/back-ldbm/ldbm_search.c | 166 ++++++++++++++----------- ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 23 +-- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 2 ldap/servers/slapd/back-ldbm/seq.c | 4 ldap/servers/slapd/back-ldbm/vlv.c | 2 15 files changed, 220 insertions(+), 195 deletions(-) New commits: commit 1118cc9b61a60c704250080d6af8785e4f8d28af Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Thu May 15 14:43:28 2014 -0700 Ticket #47780 - Some VLV search request causes memory leaks Fix description: . Modified idl_free interface as follows so that passed idl is cleared with NULL once the IDList is successfully freed. -idl_free(IDList *idl) +idl_free(IDList **idl) This change is used to clean up search candidates when ldbm_back_ search_cleanup (ldbm_search.c) is called as an error return. The cleanup function frees the search candidates when it's not NULL and it's not assigned to sr_candidates field in the search result. This fixes a memory leak when VLV/Sort op fails. . ldbm_back_search_cleanup (ldbm_search.c) calls slapi_send_ldap_result if an ldap error is passed to the function. The logic used to be "if (ldap_result>=LDAP_SUCCESS)", which is based upon that mozldap return codes are all positive. Supporting openldap library, there is a chance to get a negative return code (e.g. LDAP_PARAM_ERROR == -9). This patch supports the negative return codes, as well. . In ldbm_back_search (ldbm_search.c) vlv_filter_candidates could ruturn errors such as and LDAP_TIMELIMIT_EXCEEDED, LDAP_ADMINLIMIT_ EXCEEDED. The search results are supposed to be returned to the client with the error code if the control is not critical. The code is added. . The VLV operation stores the result in vlv_response_control.result in ldbm_back_search (ldbm_search.c), which occurs at 3 places, vlv_ filter_candidates, sort_candidates and vlv_trim_candidates_txn. The return code from the latter calls used to override the former return code. This patch fixes it to respect the former return code. https://fedorahosted.org/389/ticket/47780 Reviewed by rmeggins(a)redhat.com (Thank you, Rich!!) diff --git a/ldap/servers/slapd/back-ldbm/ancestorid.c b/ldap/servers/slapd/back-ldbm/ancestorid.c index b4d8292..8968549 100644 --- a/ldap/servers/slapd/back-ldbm/ancestorid.c +++ b/ldap/servers/slapd/back-ldbm/ancestorid.c @@ -152,7 +152,7 @@ static int ldbm_get_nonleaf_ids(backend *be, DB_TXN *txn, IDList **idl, ImportJo LDAPDebug(LDAP_DEBUG_TRACE, "found %lu nodes for ancestorid\n", (u_long)IDL_NIDS(nodes), 0, 0); } else { - idl_free(nodes); + idl_free(&nodes); *idl = NULL; } @@ -295,7 +295,7 @@ static int ldbm_ancestorid_default_create_index(backend *be, ImportJob *job) /* Insert into ancestorid for this node */ if (id2idl_hash_lookup(ht, &id, &ididl)) { descendants = idl_union_allids(be, ai_aid, ididl->idl, children); - idl_free(children); + idl_free(&children); if (id2idl_hash_remove(ht, &id) == 0) { LDAPDebug(LDAP_DEBUG_ANY, "ancestorid hash_remove failed\n", 0,0,0); } else { @@ -310,21 +310,21 @@ static int ldbm_ancestorid_default_create_index(backend *be, ImportJob *job) /* Get parentid for this entry */ ret = ldbm_parentid(be, txn, id, &parentid); if (ret != 0) { - idl_free(descendants); + idl_free(&descendants); break; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(descendants); + idl_free(&descendants); continue; } /* Insert into ancestorid for this node's parent */ if (id2idl_hash_lookup(ht, &parentid, &ididl)) { IDList *idl = idl_union_allids(be, ai_aid, ididl->idl, descendants); - idl_free(descendants); - idl_free(ididl->idl); + idl_free(&descendants); + idl_free(&(ididl->idl)); ididl->idl = idl; } else { ididl = (id2idl*)slapi_ch_calloc(1,sizeof(id2idl)); @@ -350,7 +350,7 @@ static int ldbm_ancestorid_default_create_index(backend *be, ImportJob *job) id2idl_hash_destroy(ht); /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -503,7 +503,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be, ImportJob *job) /* Insert into ancestorid for this node */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); break; } @@ -514,13 +514,13 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be, ImportJob *job) slapi_log_error(SLAPI_LOG_FATAL, sourcefile, "Error: ldbm_parentid on node index [" ID_FMT "] of [" ID_FMT "]\n", nids, nodes->b_nids); - idl_free(children); + idl_free(&children); goto out; } /* A suffix entry does not have a parent */ if (parentid == NOID) { - idl_free(children); + idl_free(&children); break; } @@ -532,7 +532,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be, ImportJob *job) /* Insert into ancestorid for this node's parent */ ret = idl_store_block(be, db_aid, &key, children, txn, ai_aid); if (ret != 0) { - idl_free(children); + idl_free(&children); goto out; } id = parentid; @@ -556,7 +556,7 @@ static int ldbm_ancestorid_new_idl_create_index(backend *be, ImportJob *job) } /* Free any leftover idlists */ - idl_free(nodes); + idl_free(&nodes); /* Release the parentid file */ if (db_pid != NULL) { @@ -635,7 +635,7 @@ static int ldbm_parentid(backend *be, DB_TXN *txn, ID id, ID *ppid) static void id2idl_free(id2idl **ididl) { - idl_free((*ididl)->idl); + idl_free(&((*ididl)->idl)); slapi_ch_free((void**)ididl); } @@ -837,7 +837,7 @@ static int ldbm_ancestorid_index_update( break; } node_id = idl_firstid(idl); - idl_free(idl); + idl_free(&idl); } /* Update ancestorid for the base entry */ diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c index 9ad2ab4..c8c40c4 100644 --- a/ldap/servers/slapd/back-ldbm/filterindex.c +++ b/ldap/servers/slapd/back-ldbm/filterindex.c @@ -399,7 +399,7 @@ presence_candidates( LDAPDebug(LDAP_DEBUG_TRACE, "fallback to eq index as pres index gave allids\n", 0, 0, 0); - idl_free(idl); + idl_free(&idl); idl = index_range_read_ext(pb, be, type, indextype_EQUALITY, SLAPI_OP_GREATER_OR_EQUAL, NULL, NULL, 0, &txn, err, allidslimit); @@ -481,7 +481,7 @@ extensible_candidates( else if (keys == NULL || keys[0] == NULL) { /* no keys */ - idl_free (idl); + idl_free (&idl); idl = idl_allids (be); } else @@ -516,8 +516,8 @@ extensible_candidates( else { IDList* tmp = idl_intersection (be, idl2, idl3); - idl_free (idl2); - idl_free (idl3); + idl_free (&idl2); + idl_free (&idl3); idl2 = tmp; } if (idl2 == NULL) break; /* look no further */ @@ -529,8 +529,8 @@ extensible_candidates( else if (idl2 != NULL) { IDList* tmp = idl_union (be, idl, idl2); - idl_free (idl); - idl_free (idl2); + idl_free (&idl); + idl_free (&idl2); idl = tmp; } } @@ -828,7 +828,7 @@ list_candidates( { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -838,7 +838,7 @@ list_candidates( == NULL && ftype == LDAP_FILTER_AND ) { LDAPDebug( LDAP_DEBUG_TRACE, "<= list_candidates NULL\n", 0, 0, 0 ); - idl_free( idl ); + idl_free( &idl ); idl = NULL; goto out; } @@ -862,15 +862,15 @@ list_candidates( int notin_result = 0; notin_result = idl_notin( be, idl, tmp, &new_idl ); if (notin_result) { - idl_free(idl); + idl_free(&idl); idl = new_idl; } } } else { idl = idl_intersection(be, idl, tmp); - idl_free( tmp2 ); + idl_free( &tmp2 ); } - idl_free( tmp ); + idl_free( &tmp ); /* stop if the list has gotten too small */ if ((idl == NULL) || (idl_length(idl) <= FILTER_TEST_THRESHOLD)) @@ -880,8 +880,8 @@ list_candidates( slapi_pblock_get( pb, SLAPI_OPERATION, &operation ); idl = idl_union( be, idl, tmp ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); /* stop if we're already committed to an exhaustive * search. :( */ @@ -890,7 +890,7 @@ list_candidates( if (op_is_pagedresults(operation)) { int nids = IDL_NIDS(idl); if ( allidslimit > 0 && nids > allidslimit ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } } @@ -1013,7 +1013,7 @@ keys2idl( } #endif if ( idl2 == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl = NULL; break; } @@ -1025,8 +1025,8 @@ keys2idl( tmp = idl; idl = idl_intersection(be, idl, idl2); - idl_free( idl2 ); - idl_free( tmp ); + idl_free( &idl2 ); + idl_free( &tmp ); if ( idl == NULL ) { break; } diff --git a/ldap/servers/slapd/back-ldbm/idl.c b/ldap/servers/slapd/back-ldbm/idl.c index a0bc139..6a7c7b0 100644 --- a/ldap/servers/slapd/back-ldbm/idl.c +++ b/ldap/servers/slapd/back-ldbm/idl.c @@ -284,12 +284,12 @@ idl_old_fetch( if ( ! INDIRECT_BLOCK( idl ) ) { /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); } - idl_free( idl ); + idl_free( &idl ); /* Taking a transaction is expensive; so we try and optimize for the common case by not taking one above. If we have a indirect block; we need to take a transaction and re-read @@ -311,7 +311,7 @@ idl_old_fetch( dblayer_read_txn_commit(be, &s_txn); /* make sure we have the current value of highest id */ if ( ALLIDS(idl) ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); } return( idl ); @@ -371,7 +371,7 @@ idl_old_fetch( dblayer_read_txn_commit(be, &s_txn); tmp[i] = NULL; slapi_ch_free((void**)&kstr ); - idl_free( idl ); + idl_free( &idl ); /* allocate space for the big block */ idl = idl_alloc( nids ); @@ -388,7 +388,7 @@ idl_old_fetch( tmp[i]->b_nids * sizeof(ID) ); nids += tmp[i]->b_nids; - idl_free( tmp[i] ); + idl_free( &tmp[i] ); } slapi_ch_free((void**)&tmp ); @@ -633,7 +633,7 @@ idl_old_insert_key( rc, (msg = dblayer_strerror( rc )) ? msg : "", 0 ); } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -659,10 +659,10 @@ idl_old_insert_key( case 3: /* id not inserted - block must be split */ /* check threshold for marking this an all-id block */ if ( a->ai_idl->idl_maxindirect < 2 ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK) @@ -677,7 +677,7 @@ idl_old_insert_key( } idl_split_block( idl, id, &tmp, &tmp2 ); - idl_free( idl ); + idl_free( &idl ); /* create the header indirect block */ idl = idl_alloc( 3 ); @@ -690,9 +690,9 @@ idl_old_insert_key( /* store it */ rc = idl_store( be, db, key, idl, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 3 BAD %d %s\n", @@ -716,9 +716,9 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( idl ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); if ( rc != DB_LOCK_DEADLOCK ) { LDAPDebug( LDAP_DEBUG_ANY, "idl_insert_key 4 BAD %d %s\n", @@ -730,12 +730,12 @@ idl_old_insert_key( "idl_insert_key", "split", key, id); slapi_ch_free((void**)&kstr ); - idl_free( tmp ); - idl_free( tmp2 ); + idl_free( &tmp ); + idl_free( &tmp2 ); break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) { @@ -763,7 +763,7 @@ idl_old_insert_key( (u_long)id, key.dptr, i); #endif idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); return( 0 ); } if ( i != 0 ) { @@ -787,7 +787,7 @@ idl_old_insert_key( LDAPDebug( LDAP_DEBUG_ANY, "nonexistent continuation block (%s)\n", k2.dptr, 0, 0 ); idl_unlock_list(a->ai_idl,key); - idl_free( idl ); + idl_free( &idl ); slapi_ch_free((void**)&kstr ); return( -1 ); } @@ -915,9 +915,9 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( tmp2 ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &tmp2 ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); @@ -945,7 +945,7 @@ idl_old_insert_key( * which is not correct. */ rc = 0; - idl_free( tmp2 ); + idl_free( &tmp2 ); break; } if ( rc != 0 ) { @@ -990,7 +990,7 @@ idl_old_insert_key( /* store allid block in place of header block */ if ( 0 == rc ) { - idl_free( idl ); + idl_free( &idl ); idl = idl_allids( be ); rc = idl_store( be, db, key, idl, txn ); if (NULL != disposition) { @@ -1000,14 +1000,14 @@ idl_old_insert_key( slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( idl ); - idl_free( tmp ); + idl_free( &idl ); + idl_free( &tmp ); idl_unlock_list(a->ai_idl,key); return( rc ); } idl_split_block( tmp, id, &tmp2, &tmp3 ); - idl_free( tmp ); + idl_free( &tmp ); /* create a new updated indirect header block */ tmp = idl_alloc( idl->b_nmax + 1 ); @@ -1025,8 +1025,8 @@ idl_old_insert_key( /* store the header block */ rc = idl_store( be, db, key, tmp, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1037,8 +1037,8 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp2, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } @@ -1049,22 +1049,22 @@ idl_old_insert_key( k2.dsize = strlen( kstr ) + 1; rc = idl_store( be, db, &k2, tmp3, txn ); if ( rc != 0 ) { - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } idl_check_indirect (tmp, i, tmp2, tmp3, "idl_insert_key", "indirect split", key, id); - idl_free( tmp2 ); - idl_free( tmp3 ); + idl_free( &tmp2 ); + idl_free( &tmp3 ); break; } slapi_ch_free( (void **)&(k2.dptr) ); slapi_ch_free( (void **)&(k3.dptr) ); - idl_free( tmp ); - idl_free( idl ); + idl_free( &tmp ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); return( rc ); } @@ -1100,7 +1100,7 @@ int idl_old_store_block( /* If so, store an ALLIDS block */ IDList *all = idl_allids(be); ret = idl_store(be,db,key,all,txn); - idl_free(all); + idl_free(&all); } else { /* Then , is it a block which is smaller than the size at which it needs splitting ? */ if (idl->b_nids <= (ID)priv->idl_maxids) { @@ -1159,7 +1159,7 @@ int idl_old_store_block( make_cont_key(&cont_key,key,lead_id); /* Now store the continuation block */ ret = idl_store(be,db,&cont_key,this_cont_block,txn); - idl_free(this_cont_block); + idl_free(&this_cont_block); slapi_ch_free(&(cont_key.data)); if ( ret != 0 && ret != DB_LOCK_DEADLOCK ) { @@ -1181,7 +1181,7 @@ int idl_old_store_block( } done: /* Free master block */ - idl_free(master_block); + idl_free(&master_block); return ret; } @@ -1442,7 +1442,7 @@ idl_old_delete_key( break; } - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) %d (not indirect)\n", key->dptr, (u_long)id, rc ); @@ -1463,7 +1463,7 @@ idl_old_delete_key( } /* id smaller than smallest id - not there */ if ( i == 0 && id < idl->b_ids[i] ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); LDAPDebug( LDAP_DEBUG_TRACE, "<= idl_delete_key(%s,%lu) -666 (id not found)\n", key->dptr, (u_long)id, 0 ); @@ -1476,7 +1476,7 @@ idl_old_delete_key( /* get the block to delete from */ make_cont_key( &contkey, key, idl->b_ids[i] ); if ( (didl = idl_fetch_one( li, db, &contkey, txn, &rc )) == NULL ) { - idl_free( idl ); + idl_free( &idl ); idl_unlock_list(a->ai_idl,key); if ( rc != DB_LOCK_DEADLOCK ) { @@ -1563,8 +1563,8 @@ idl_old_delete_key( rc = 0; break; } - idl_free( idl ); - idl_free( didl ); + idl_free( &idl ); + idl_free( &didl ); slapi_ch_free( (void **)&(contkey.dptr) ); idl_unlock_list(a->ai_idl,key); if ( rc != 0 && rc != DB_LOCK_DEADLOCK ) diff --git a/ldap/servers/slapd/back-ldbm/idl_common.c b/ldap/servers/slapd/back-ldbm/idl_common.c index b6d2e68..97b2e36 100644 --- a/ldap/servers/slapd/back-ldbm/idl_common.c +++ b/ldap/servers/slapd/back-ldbm/idl_common.c @@ -94,13 +94,13 @@ idl_allids( backend *be ) } void -idl_free( IDList *idl ) /* JCM - pass in ** */ +idl_free( IDList **idl ) { - if ( idl == NULL ) { + if ((NULL == idl) || (NULL == *idl)) { return; } - slapi_ch_free((void**)&idl ); + slapi_ch_free((void**)idl); } @@ -163,7 +163,7 @@ idl_append_extend(IDList **orig_idl, ID id) /* copy over the existing contents */ idl_new->b_nids = idl->b_nids; memcpy(idl_new->b_ids, idl->b_ids, sizeof(ID) * idl->b_nids); - idl_free(idl); + idl_free(&idl); idl = idl_new; } @@ -255,7 +255,7 @@ idl_intersection( } if ( ni == 0 ) { - idl_free( n ); + idl_free( &n ); return( NULL ); } n->b_nids = ni; @@ -367,7 +367,7 @@ idl_notin( } if ( ni == n->b_nmax ) { - idl_free( n ); + idl_free( &n ); *new_result = idl_allids( be ); } else { n->b_nids = ni; diff --git a/ldap/servers/slapd/back-ldbm/idl_new.c b/ldap/servers/slapd/back-ldbm/idl_new.c index f0410f9..120153e 100644 --- a/ldap/servers/slapd/back-ldbm/idl_new.c +++ b/ldap/servers/slapd/back-ldbm/idl_new.c @@ -285,7 +285,7 @@ idl_new_fetch( idl_rc = idl_append_extend(&idl, id); if (idl_rc) { LDAPDebug(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc, 0, 0); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -323,7 +323,7 @@ idl_new_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -339,7 +339,7 @@ idl_new_fetch( #endif if (ret != DB_NOTFOUND) { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -348,7 +348,7 @@ idl_new_fetch( /* check for allids value */ if (idl != NULL && idl->b_nids == 1 && idl->b_ids[0] == ALLID) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", key.data, 0, 0); @@ -501,7 +501,7 @@ idl_new_range_fetch( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "idl_new_range_fetch - lookthrough_limit exceeded\n"); @@ -557,7 +557,7 @@ idl_new_range_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } @@ -618,7 +618,7 @@ idl_new_range_fetch( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "idl_new_range_fetch - lookthrough_limit exceeded\n"); @@ -669,7 +669,7 @@ idl_new_range_fetch( if (idl_rc) { LDAPDebug1Arg(LDAP_DEBUG_ANY, "unable to extend id list (err=%d)\n", idl_rc); - idl_free(idl); idl = NULL; + idl_free(&idl); goto error; } #if defined(DB_ALLIDS_ON_READ) @@ -688,7 +688,7 @@ idl_new_range_fetch( if (ret == DB_NOTFOUND) { ret = 0; /* normal case */ } else { - idl_free(idl); idl = NULL; + idl_free(&idl); ldbm_nasty(filename,59,ret); goto error; } @@ -696,7 +696,7 @@ idl_new_range_fetch( /* check for allids value */ if (idl && (idl->b_nids == 1) && (idl->b_ids[0] == ALLID)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids(be); LDAPDebug1Arg(LDAP_DEBUG_TRACE, "idl_new_fetch %s returns allids\n", cur_key.data); diff --git a/ldap/servers/slapd/back-ldbm/import-merge.c b/ldap/servers/slapd/back-ldbm/import-merge.c index 223f34f..a728165 100644 --- a/ldap/servers/slapd/back-ldbm/import-merge.c +++ b/ldap/servers/slapd/back-ldbm/import-merge.c @@ -169,8 +169,8 @@ static int import_merge_insert_input_queue(backend *be, import_merge_queue_entry IDList *new_idl = idl_union(be, current_entry->thang.payload.idl, idl); - idl_free(current_entry->thang.payload.idl); - idl_free(idl); + idl_free(&(current_entry->thang.payload.idl)); + idl_free(&idl); current_entry->thang.payload.idl = new_idl; /* Add this file id into the entry's referenced list */ (current_entry->file_referenced_list)[fileno] = 1; @@ -567,7 +567,7 @@ static int import_merge_one_file(ImportWorkerInfo *worker, int passes, ret = idl_store_block(be, output_file, &key, thang.payload.idl, NULL, worker->index_info->ai); /* Free the key we got back from the queue */ - idl_free(thang.payload.idl); + idl_free(&(thang.payload.idl)); thang.payload.idl = NULL; } slapi_ch_free(&(key.data)); diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c index 1e9f763..8344cfd 100644 --- a/ldap/servers/slapd/back-ldbm/import-threads.c +++ b/ldap/servers/slapd/back-ldbm/import-threads.c @@ -2355,7 +2355,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) */ if (IDL) { ID id = idl_firstid(IDL); /* entrydn is a single attr */ - idl_free(IDL); + idl_free(&IDL); if (id != entry->ep_id) { /* case (2) */ import_log_notice(job, "Duplicated entrydn detected: \"%s\": " "Entry ID: (%d, %d)", @@ -2381,7 +2381,7 @@ foreman_do_entrydn(ImportJob *job, FifoItem *fi) "\"%s\" found at line %d of file \"%s\"", slapi_entry_get_dn(entry->ep_entry), fi->line, fi->filename); - idl_free(IDL); + idl_free(&IDL); /* skip this one */ fi->bad = FIFOITEM_BAD; job->skipped++; diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c index 5610b49..ffeedef 100644 --- a/ldap/servers/slapd/back-ldbm/index.c +++ b/ldap/servers/slapd/back-ldbm/index.c @@ -197,7 +197,7 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) goto error; } slapi_ch_free( &(bin->key.data) ); - idl_free(bin->value); + idl_free(&(bin->value)); /* If we're already at allids, store an allids block to prevent needless accumulation of blocks */ if (old_idl && ALLIDS(old_idl)) { bin->value = idl_allids(be); @@ -207,10 +207,10 @@ index_put_idl(index_buffer_bin *bin,backend *be, DB_TXN *txn,struct attrinfo *a) } error: if (old_idl) { - idl_free(old_idl); + idl_free(&old_idl); } if (new_idl && need_to_freed_new_idl) { - idl_free(new_idl); + idl_free(&new_idl); } dblayer_release_index_file( be, a, db ); return ret; @@ -268,7 +268,7 @@ index_buffer_terminate(void *h) for (i = 0; i < handle->buffer_size; i++) { bin = &(handle->bins[i]); if (bin->value) { - idl_free(bin->value); + idl_free(&(bin->value)); bin->value = NULL; } slapi_ch_free(&(bin->key.data)); @@ -1505,7 +1505,7 @@ index_range_read_ext( if (idl) { if ((lookthrough_limit != -1) && (idl->b_nids > (ID)lookthrough_limit)) { - idl_free(idl); + idl_free(&idl); idl = idl_allids( be ); LDAPDebug0Args(LDAP_DEBUG_TRACE, "index_range_read lookthrough_limit exceeded\n"); @@ -1534,7 +1534,7 @@ index_range_read_ext( */ if ( slapi_op_abandoned( pb )) { if (NULL != idl) { - idl_free(idl); + idl_free(&idl); idl = NULL; } LDAPDebug0Args(LDAP_DEBUG_TRACE, @@ -1574,7 +1574,7 @@ index_range_read_ext( /* idl tmp only contains one id */ /* append it at the end here; sort idlist at the end */ if (ALLIDS(tmp)) { - idl_free(idl); + idl_free(&idl); idl = tmp; } else { ID id; @@ -1586,7 +1586,7 @@ index_range_read_ext( 1097, *err); } } - idl_free(tmp); + idl_free(&tmp); } if (ALLIDS(idl)) { LDAPDebug0Args(LDAP_DEBUG_TRACE, diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c index 46d30eb..d4039e4 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c @@ -1100,7 +1100,7 @@ entryrdn_get_subordinates(backend *be, bail: if (rc && subordinates && *subordinates) { - idl_free(*subordinates); + idl_free(subordinates); } slapi_ch_free_string(&keybuf); slapi_ch_free((void **)&elem); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index b1b6cc9..472c2f7 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -1524,7 +1524,7 @@ common_return: slapi_ch_free((void**)&child_dns); if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL")) slapi_ch_free((void**)&ldap_result_matcheddn); - idl_free(children); + idl_free(&children); slapi_sdn_done(&dn_newdn); slapi_sdn_done(&dn_newrdn); slapi_sdn_done(&dn_parentdn); @@ -2238,7 +2238,7 @@ moddn_get_children(back_txn *ptxn, } } } while (id!=NOID); - idl_free(candidates); + idl_free(&candidates); slapi_sdn_done(&parentsdn); } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index d0563bd..6dff1f5 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -48,6 +48,16 @@ #include "back-ldbm.h" #include "vlv_srch.h" +/* + * Used for ldap_result passed to ldbm_back_search_cleanup. + * If (ldap_result == LDBM_SRCH_DEFAULT_RESULT) || (ldap_result == LDAP_SUCCESS), + * don't call slapi_send_ldap_result. + * Note: mozldap ldap_result codes are all positive; openldap result codes could + * be negative values. OL (-1) is LDAP_SERVER_DOWN. Thus, it's safe to + * borrow the value here. + */ +#define LDBM_SRCH_DEFAULT_RESULT (-1) + /* prototypes */ static int build_candidate_list( Slapi_PBlock *pb, backend *be, struct backentry *e, const char * base, int scope, @@ -162,12 +172,14 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, char* ldap_result_description, int function_result, struct vlv_request *vlv_request_control, - struct backentry *e) + struct backentry *e, + IDList *candidates) { int estimate = 0; /* estimated search result count */ backend *be; ldbm_instance *inst; back_search_result_set *sr = NULL; + int free_candidates = 1; slapi_pblock_get( pb, SLAPI_BACKEND, &be ); inst = (ldbm_instance *) be->be_instance_info; @@ -180,20 +192,27 @@ ldbm_back_search_cleanup(Slapi_PBlock *pb, { sort_spec_free(sort_control); } - if(ldap_result>=LDAP_SUCCESS) - { + if ((ldap_result != LDBM_SRCH_DEFAULT_RESULT) && (ldap_result != LDAP_SUCCESS)) { slapi_send_ldap_result( pb, ldap_result, NULL, ldap_result_description, 0, NULL ); } /* code to free the result set if we don't need it */ /* We get it and check to see if the structure was ever used */ slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr); if (sr) { - if (function_result) { + if (function_result) { /* failed case */ slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_SET_SIZE_ESTIMATE, &estimate); slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL); + if (sr->sr_candidates == candidates) { + free_candidates = 0; + } delete_search_result_set(pb, &sr); + } else if (sr->sr_candidates == candidates) { /* succeeded case */ + free_candidates = 0; } } + if (free_candidates) { + idl_free(&candidates); + } if (vlv_request_control) { berval_done(&vlv_request_control->value); @@ -336,7 +355,7 @@ ldbm_back_search( Slapi_PBlock *pb ) back_search_result_set *sr = NULL; /* Fix for bugid #394184, SD, 20 Jul 00 */ - int tmp_err = -1; /* must be lower than LDAP_SUCCESS */ + int tmp_err = LDBM_SRCH_DEFAULT_RESULT; char * tmp_desc = NULL; /* end Fix for defect #394184 */ @@ -401,7 +420,7 @@ ldbm_back_search( Slapi_PBlock *pb ) */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "Sort Control", - SLAPI_FAIL_GENERAL, NULL, NULL); + SLAPI_FAIL_GENERAL, NULL, NULL, candidates); } else { PRUint64 conn_id; int op_id; @@ -436,7 +455,7 @@ ldbm_back_search( Slapi_PBlock *pb ) */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + &vlv_request_control, NULL, candidates); } else { PRUint64 conn_id; int op_id; @@ -470,7 +489,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Client isn't allowed to do this. */ return ldbm_back_search_cleanup(pb, li, sort_control, rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, NULL); + &vlv_request_control, NULL, candidates); } } /* @@ -485,7 +504,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Can't have a VLV control without a SORT control */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_SORT_CONTROL_MISSING, "VLV Control", - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } } @@ -538,13 +557,13 @@ ldbm_back_search( Slapi_PBlock *pb ) { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNWILLING_TO_PERFORM, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else { return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_VIRTUAL_LIST_VIEW_ERROR, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } } else @@ -559,7 +578,7 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_make_sort_response_control(pb, LDAP_UNWILLING_TO_PERFORM, NULL); return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_UNAVAILABLE_CRITICAL_EXTENSION, ctrlstr, - SLAPI_FAIL_GENERAL, &vlv_request_control, NULL); + SLAPI_FAIL_GENERAL, &vlv_request_control, NULL, candidates); } else /* vlv and sorting are not critical, so ignore the control */ { @@ -593,7 +612,7 @@ ldbm_back_search( Slapi_PBlock *pb ) { /* error or referral sent by find_entry */ return ldbm_back_search_cleanup(pb, li, sort_control, - -1, NULL, 1, &vlv_request_control, NULL); + LDBM_SRCH_DEFAULT_RESULT, NULL, 1, &vlv_request_control, NULL, candidates); } } @@ -616,6 +635,7 @@ ldbm_back_search( Slapi_PBlock *pb ) /* * Build a list of IDs for this entry and scope */ + vlv_response_control.result = LDAP_SUCCESS; if ((NULL != controls) && (sort) && (vlv)) { /* This candidate list is for vlv, no need for sort only. */ switch (vlv_search_build_candidate_list(pb, basesdn, &vlv_rc, @@ -626,12 +646,12 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_rc, "VLV Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case VLV_BLD_LIST_FAILED: return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_SUCCESS: /* Log to the access log the particulars of this sort request */ @@ -652,7 +672,7 @@ ldbm_back_search( Slapi_PBlock *pb ) LDAP_OPERATIONS_ERROR, "Sort Response Control", SLAPI_FAIL_GENERAL, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -663,9 +683,9 @@ ldbm_back_search( Slapi_PBlock *pb ) if (rc) { /* Error result sent by build_candidate_list */ - return ldbm_back_search_cleanup(pb, li, sort_control, -1, - NULL, rc, - &vlv_request_control, e); + return ldbm_back_search_cleanup(pb, li, sort_control, + LDBM_SRCH_DEFAULT_RESULT, NULL, rc, + &vlv_request_control, e, candidates); } /* * If we're sorting then we must check what administrative @@ -691,23 +711,33 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then apply the * search filter before sorting. */ - if (virtual_list_view && (NULL != candidates)) - { + if (virtual_list_view && candidates) { IDList *idl = NULL; Slapi_Filter *filter = NULL; slapi_pblock_get( pb, SLAPI_SEARCH_FILTER, &filter ); rc = vlv_filter_candidates(be, pb, candidates, basesdn, - scope, filter, &idl, - lookthrough_limit, time_up); - if (rc == 0) { - idl_free(candidates); - candidates= idl; - } - else - { + scope, filter, &idl, + lookthrough_limit, time_up); + switch (rc) { + case LDAP_SUCCESS: /* Everything OK */ + case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ + case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ + vlv_response_control.result = rc; + idl_free(&candidates); + candidates = idl; + break; + case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ + default: return ldbm_back_search_cleanup(pb, li, sort_control, rc, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); + } + if (is_vlv_critical && rc) { + idl_free(&candidates); + candidates = idl_alloc(0); + tmp_err = rc; + tmp_desc = "VLV Response Control"; + goto vlv_bail; } } /* @@ -725,7 +755,7 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } else @@ -759,21 +789,24 @@ ldbm_back_search( Slapi_PBlock *pb ) /* replace the hard coded return value by the appropriate * LDAP error code */ switch (sort_return_value) { - case LDAP_SUCCESS: /* Everything OK */ - vlv_response_control.result= LDAP_SUCCESS; + case LDAP_SUCCESS: + /* + * we don't want to override an error from vlv + * vlv_response_control.result= LDAP_SUCCESS; + */ break; case LDAP_PROTOCOL_ERROR: /* A protocol error */ return ldbm_back_search_cleanup(pb, li, sort_control, LDAP_PROTOCOL_ERROR, "Sort Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); case LDAP_UNWILLING_TO_PERFORM: /* Too hard */ case LDAP_OPERATIONS_ERROR: /* Operation error */ case LDAP_TIMELIMIT_EXCEEDED: /* Timeout */ - vlv_response_control.result= LDAP_TIMELIMIT_EXCEEDED; - break; case LDAP_ADMINLIMIT_EXCEEDED: /* Admin limit exceeded */ - vlv_response_control.result = LDAP_ADMINLIMIT_EXCEEDED; + if (!vlv_response_control.result) { + vlv_response_control.result = sort_return_value; + } break; case LDAP_OTHER: /* Abandoned */ abandoned = 1; /* So that we don't return a result code */ @@ -792,11 +825,10 @@ ldbm_back_search( Slapi_PBlock *pb ) /* Fix for bugid #394184, SD, 05 Jul 00 */ /* we were not actually returning unavailableCriticalExtension; now fixed (hopefully !) */ - if (is_sorting_critical && (0 != sort_return_value)) - { - idl_free(candidates); + if (is_sorting_critical && sort_return_value) { + idl_free(&candidates); candidates = idl_alloc(0); - tmp_err = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + tmp_err = sort_return_value; tmp_desc = "Sort Response Control"; } /* end Fix for bugid #394184 */ @@ -806,9 +838,9 @@ ldbm_back_search( Slapi_PBlock *pb ) sort_return_value, sort_error_type ) ) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "Sort Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } } } @@ -816,10 +848,9 @@ ldbm_back_search( Slapi_PBlock *pb ) * If we're presenting a virtual list view, then the candidate list * must be trimmed down to just the range of entries requested. */ - if (virtual_list_view) - { - if (NULL != candidates && candidates->b_nids>0) - { + if (virtual_list_view) { + if (candidates && (candidates->b_nids > 0) && + !vlv_response_control.result) { IDList *idl= NULL; back_txn txn = {NULL}; slapi_pblock_get(pb, SLAPI_TXN, &txn.back_txn_txn); @@ -828,7 +859,7 @@ ldbm_back_search( Slapi_PBlock *pb ) &vlv_request_control, &idl, &vlv_response_control, &txn); if(vlv_response_control.result==0) { - idl_free(candidates); + idl_free(&candidates); candidates = idl; } else @@ -836,26 +867,23 @@ ldbm_back_search( Slapi_PBlock *pb ) return ldbm_back_search_cleanup(pb, li, sort_control, vlv_response_control.result, NULL, -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } - } - else - { + } else { vlv_response_control.targetPosition = 0; vlv_response_control.contentCount = 0; - vlv_response_control.result = LDAP_SUCCESS; + /* vlv_response_control.result = LDAP_SUCCESS; Don't override */ } } } - if (virtual_list_view) - { - if(LDAP_SUCCESS != - vlv_make_response_control( pb, &vlv_response_control )) - { +vlv_bail: + if (virtual_list_view) { + if (LDAP_SUCCESS != + vlv_make_response_control( pb, &vlv_response_control )) { return ldbm_back_search_cleanup(pb, li, sort_control, - (abandoned?-1:LDAP_PROTOCOL_ERROR), + (abandoned?LDBM_SRCH_DEFAULT_RESULT:LDAP_PROTOCOL_ERROR), "VLV Response Control", -1, - &vlv_request_control, e); + &vlv_request_control, e, candidates); } /* Log the VLV operation */ vlv_print_access_log(pb,&vlv_request_control,&vlv_response_control); @@ -885,7 +913,7 @@ ldbm_back_search( Slapi_PBlock *pb ) PR_Unlock(inst->inst_config_mutex); if (ri) { - idl_free(candidates); + idl_free(&candidates); candidates = idl_alloc(0); tmp_err = LDAP_UNWILLING_TO_PERFORM; tmp_desc = "Search is not indexed"; @@ -948,10 +976,10 @@ ldbm_back_search( Slapi_PBlock *pb ) } /* Fix for bugid #394184, SD, 05 Jul 00 */ - /* tmp_err == -1: no error */ + /* tmp_err == LDBM_SRCH_DEFAULT_RESULT: no error */ return ldbm_back_search_cleanup(pb, li, sort_control, tmp_err, tmp_desc, - (tmp_err == -1 ? 0 : -1), - &vlv_request_control, NULL); + (tmp_err == LDBM_SRCH_DEFAULT_RESULT ? 0 : LDBM_SRCH_DEFAULT_RESULT), + &vlv_request_control, NULL, candidates); /* end Fix for bugid #394184 */ } @@ -1212,14 +1240,14 @@ subtree_candidates( e->ep_id, &descendants, &txn, 0); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } else if (!has_tombstone_filter) { *err = ldbm_ancestorid_read_ext(be, &txn, e->ep_id, &descendants, allidslimit); idl_insert(&descendants, e->ep_id); candidates = idl_intersection(be, candidates, descendants); - idl_free(tmp); - idl_free(descendants); + idl_free(&tmp); + idl_free(&descendants); } /* else == has_tombstone_filter: do nothing */ } @@ -1847,7 +1875,7 @@ delete_search_result_set( Slapi_PBlock *pb, back_search_result_set **sr ) } if ( NULL != (*sr)->sr_candidates ) { - idl_free( (*sr)->sr_candidates ); + idl_free( &((*sr)->sr_candidates) ); } rc = slapi_filter_apply((*sr)->sr_norm_filter, ldbm_search_free_compiled_filter, NULL, &filt_errs); diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c index c4c9414..1d1b2dc 100644 --- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c +++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c @@ -292,7 +292,7 @@ add_op_attrs(Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *ep, if ( (idl = index_read( be, LDBM_ENTRYDN_STR, indextype_EQUALITY, &bv, NULL, &err )) != NULL ) { pid = idl_firstid( idl ); - idl_free( idl ); + idl_free( &idl ); } else { /* empty idl */ if ( 0 != err && DB_NOTFOUND != err ) { @@ -609,7 +609,7 @@ int update_subordinatecounts(backend *be, ImportJob *job, DB_TXN *txn) /* We get the count from the IDL */ sub_count = idl->b_nids; } - idl_free(idl); + idl_free(&idl); } /* Did we get the count ? */ if (found_count) { @@ -914,8 +914,7 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) continue; } id = idl_firstid(idl); - idl_free(idl); - idl = NULL; + idl_free(&idl); } /* @@ -953,8 +952,8 @@ static IDList *ldbm_fetch_subtrees(backend *be, char **include, int *err) idltotal = idl; } else if (idl) { idltmp = idl_union(be, idltotal, idl); - idl_free(idltotal); - idl_free(idl); + idl_free(&idltotal); + idl_free(&idl); idltotal = idltmp; } } /* for (i = 0; include[i]; i++) */ @@ -1356,8 +1355,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) /* allids list is no help at all -- revert to trawling * the whole list. */ ok_index = 0; - idl_free(idl); - idl = NULL; + idl_free(&idl); } idindex = 0; } @@ -1613,7 +1611,7 @@ ldbm_back_ldbm2ldif( Slapi_PBlock *pb ) } bye: if (idl) { - idl_free(idl); + idl_free(&idl); } if (dbc) { dbc->c_close(dbc); @@ -1644,7 +1642,7 @@ bye: } ldbm_back_free_incl_excl(include_suffix, exclude_suffix); - idl_free(eargs.pre_exported_idl); + idl_free(&(eargs.pre_exported_idl)); return( return_value ); } @@ -1958,8 +1956,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) } } else if (ALLIDS(idl)) { /* that's no help. */ - idl_free(idl); - idl = NULL; + idl_free(&idl); } } @@ -2468,7 +2465,7 @@ ldbm_back_ldbm2index(Slapi_PBlock *pb) err_out: backentry_free( &ep ); /* if ep or *ep is NULL, it does nothing */ if (idl) { - idl_free(idl); + idl_free(&idl); } else { dbc->c_close(dbc); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 3870ce5..561a208 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -243,7 +243,7 @@ struct backentry * id2entry( backend *be, ID id, back_txn *txn, int *err ); * idl.c */ IDList * idl_alloc( NIDS nids ); -void idl_free( IDList *idl ); +void idl_free( IDList **idl ); NIDS idl_length(IDList *idl); int idl_is_allids(IDList *idl); int idl_append(IDList *idl, ID id); diff --git a/ldap/servers/slapd/back-ldbm/seq.c b/ldap/servers/slapd/back-ldbm/seq.c index a75879c..29b3f6a 100644 --- a/ldap/servers/slapd/back-ldbm/seq.c +++ b/ldap/servers/slapd/back-ldbm/seq.c @@ -245,7 +245,7 @@ retry: key.flags = 0; for (retry_count = 0; retry_count < IDL_FETCH_RETRY_COUNT; retry_count++) { err = NEW_IDL_DEFAULT; - idl_free(idl); + idl_free(&idl); idl = idl_fetch( be, db, &key, parent_txn, ai, &err ); if(err == DB_LOCK_DEADLOCK) { ldbm_nasty("ldbm_back_seq deadlock retry", 1600, err); @@ -319,7 +319,7 @@ retry: } CACHE_RETURN( &inst->inst_cache, &e ); } - idl_free( idl ); + idl_free( &idl ); } dblayer_release_index_file( be, ai, db ); diff --git a/ldap/servers/slapd/back-ldbm/vlv.c b/ldap/servers/slapd/back-ldbm/vlv.c index 83b321a..d061bf2 100644 --- a/ldap/servers/slapd/back-ldbm/vlv.c +++ b/ldap/servers/slapd/back-ldbm/vlv.c @@ -1122,7 +1122,7 @@ int vlv_build_idl(PRUint32 start, PRUint32 stop, DB *db, DBC *dbc, goto done; error: - if (idl) idl_free(idl); + if (idl) idl_free(&idl); done: return rc;

1 0

Branch '389-ds-base-1.2.11' - dirsrvtests/tickets ldap/servers
by thierry bordaz 13 May '14

13 May '14

dirsrvtests/tickets/ticket47787_test.py | 630 +++++++++++++++++++++++++++++ ldap/servers/slapd/back-ldbm/ldbm_modify.c | 2 2 files changed, 631 insertions(+), 1 deletion(-) New commits: commit 0ee8496d0e884f788041cc2e54279cfc28eabaa9 Author: Thierry bordaz (tbordaz) <tbordaz(a)redhat.com> Date: Fri Apr 25 15:07:08 2014 +0200 Ticket 47787 - A replicated MOD fails (Unwilling to perform) if it targets a tombstone Bug Description: A fix for https://fedorahosted.org/389/ticket/47396 prevents a MOD operation on a tombstone. A problem is that legacy version may allowed it or if a MOD entry and DEL entry occurs on different master, it may happen that a consumer receive a MOD on a tombstone. The server return 'Unwilling to perform' that may break replication. Fix Description: Allows a MOD on tombstone, only if it comes from a replication session https://fedorahosted.org/389/ticket/47787 Reviewed by: Rich Megginson (thanks) Platforms tested: F17, F19 Flag Day: no Doc impact: no diff --git a/dirsrvtests/tickets/ticket47787_test.py b/dirsrvtests/tickets/ticket47787_test.py new file mode 100644 index 0000000..d9d3ca9 --- /dev/null +++ b/dirsrvtests/tickets/ticket47787_test.py @@ -0,0 +1,630 @@ +''' +Created on April 14, 2014 + +@author: tbordaz +''' +import os +import sys +import time +import ldap +import logging +import socket +import time +import logging +import pytest +import re +from lib389 import DirSrv, Entry, tools, NoSuchEntryError +from lib389.tools import DirSrvTools +from lib389._constants import * +from lib389.properties import * +from constants import * +from lib389._constants import REPLICAROLE_MASTER + +logging.getLogger(__name__).setLevel(logging.DEBUG) +log = logging.getLogger(__name__) + +# +# important part. We can deploy Master1 and Master2 on different versions +# +installation1_prefix = None +installation2_prefix = None + +# set this flag to False so that it will assert on failure _status_entry_both_server +DEBUG_FLAG = False + +TEST_REPL_DN = "cn=test_repl, %s" % SUFFIX + +STAGING_CN = "staged user" +PRODUCTION_CN = "accounts" +EXCEPT_CN = "excepts" + +STAGING_DN = "cn=%s,%s" % (STAGING_CN, SUFFIX) +PRODUCTION_DN = "cn=%s,%s" % (PRODUCTION_CN, SUFFIX) +PROD_EXCEPT_DN = "cn=%s,%s" % (EXCEPT_CN, PRODUCTION_DN) + +STAGING_PATTERN = "cn=%s*,%s" % (STAGING_CN[:2], SUFFIX) +PRODUCTION_PATTERN = "cn=%s*,%s" % (PRODUCTION_CN[:2], SUFFIX) +BAD_STAGING_PATTERN = "cn=bad*,%s" % (SUFFIX) +BAD_PRODUCTION_PATTERN = "cn=bad*,%s" % (SUFFIX) + +BIND_CN = "bind_entry" +BIND_DN = "cn=%s,%s" % (BIND_CN, SUFFIX) +BIND_PW = "password" + +NEW_ACCOUNT = "new_account" +MAX_ACCOUNTS = 20 + +CONFIG_MODDN_ACI_ATTR = "nsslapd-moddn-aci" + +class TopologyMaster1Master2(object): + def __init__(self, master1, master2): + master1.open() + self.master1 = master1 + + master2.open() + self.master2 = master2 + + +(a)pytest.fixture(scope="module") +def topology(request): + ''' + This fixture is used to create a replicated topology for the 'module'. + The replicated topology is MASTER1 <-> Master2. + At the beginning, It may exists a master2 instance and/or a master2 instance. + It may also exists a backup for the master1 and/or the master2. + + Principle: + If master1 instance exists: + restart it + If master2 instance exists: + restart it + If backup of master1 AND backup of master2 exists: + create or rebind to master1 + create or rebind to master2 + + restore master1 from backup + restore master2 from backup + else: + Cleanup everything + remove instances + remove backups + Create instances + Initialize replication + Create backups + ''' + global installation1_prefix + global installation2_prefix + + # allocate master1 on a given deployement + master1 = DirSrv(verbose=False) + if installation1_prefix: + args_instance[SER_DEPLOYED_DIR] = installation1_prefix + + # Args for the master1 instance + args_instance[SER_HOST] = HOST_MASTER_1 + args_instance[SER_PORT] = PORT_MASTER_1 + args_instance[SER_SERVERID_PROP] = SERVERID_MASTER_1 + args_master = args_instance.copy() + master1.allocate(args_master) + + # allocate master1 on a given deployement + master2 = DirSrv(verbose=False) + if installation2_prefix: + args_instance[SER_DEPLOYED_DIR] = installation2_prefix + + # Args for the consumer instance + args_instance[SER_HOST] = HOST_MASTER_2 + args_instance[SER_PORT] = PORT_MASTER_2 + args_instance[SER_SERVERID_PROP] = SERVERID_MASTER_2 + args_master = args_instance.copy() + master2.allocate(args_master) + + + # Get the status of the backups + backup_master1 = master1.checkBackupFS() + backup_master2 = master2.checkBackupFS() + + # Get the status of the instance and restart it if it exists + instance_master1 = master1.exists() + if instance_master1: + master1.stop(timeout=10) + master1.start(timeout=10) + + instance_master2 = master2.exists() + if instance_master2: + master2.stop(timeout=10) + master2.start(timeout=10) + + if backup_master1 and backup_master2: + # The backups exist, assuming they are correct + # we just re-init the instances with them + if not instance_master1: + master1.create() + # Used to retrieve configuration information (dbdir, confdir...) + master1.open() + + if not instance_master2: + master2.create() + # Used to retrieve configuration information (dbdir, confdir...) + master2.open() + + # restore master1 from backup + master1.stop(timeout=10) + master1.restoreFS(backup_master1) + master1.start(timeout=10) + + # restore master2 from backup + master2.stop(timeout=10) + master2.restoreFS(backup_master2) + master2.start(timeout=10) + else: + # We should be here only in two conditions + # - This is the first time a test involve master-consumer + # so we need to create everything + # - Something weird happened (instance/backup destroyed) + # so we discard everything and recreate all + + # Remove all the backups. So even if we have a specific backup file + # (e.g backup_master) we clear all backups that an instance my have created + if backup_master1: + master1.clearBackupFS() + if backup_master2: + master2.clearBackupFS() + + # Remove all the instances + if instance_master1: + master1.delete() + if instance_master2: + master2.delete() + + # Create the instances + master1.create() + master1.open() + master2.create() + master2.open() + + # + # Now prepare the Master-Consumer topology + # + # First Enable replication + master1.replica.enableReplication(suffix=SUFFIX, role=REPLICAROLE_MASTER, replicaId=REPLICAID_MASTER_1) + master2.replica.enableReplication(suffix=SUFFIX, role=REPLICAROLE_MASTER, replicaId=REPLICAID_MASTER_2) + + # Initialize the supplier->consumer + + properties = {RA_NAME: r'meTo_$host:$port', + RA_BINDDN: defaultProperties[REPLICATION_BIND_DN], + RA_BINDPW: defaultProperties[REPLICATION_BIND_PW], + RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD], + RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + repl_agreement = master1.agreement.create(suffix=SUFFIX, host=master2.host, port=master2.port, properties=properties) + + if not repl_agreement: + log.fatal("Fail to create a replica agreement") + sys.exit(1) + + log.debug("%s created" % repl_agreement) + + properties = {RA_NAME: r'meTo_$host:$port', + RA_BINDDN: defaultProperties[REPLICATION_BIND_DN], + RA_BINDPW: defaultProperties[REPLICATION_BIND_PW], + RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD], + RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + master2.agreement.create(suffix=SUFFIX, host=master1.host, port=master1.port, properties=properties) + + master1.agreement.init(SUFFIX, HOST_MASTER_2, PORT_MASTER_2) + master1.waitForReplInit(repl_agreement) + + # Check replication is working fine + master1.add_s(Entry((TEST_REPL_DN, { + 'objectclass': "top person".split(), + 'sn': 'test_repl', + 'cn': 'test_repl'}))) + loop = 0 + while loop <= 10: + try: + ent = master2.getEntry(TEST_REPL_DN, ldap.SCOPE_BASE, "(objectclass=*)") + break + except ldap.NO_SUCH_OBJECT: + time.sleep(1) + loop += 1 + + # Time to create the backups + master1.stop(timeout=10) + master1.backupfile = master1.backupFS() + master1.start(timeout=10) + + master2.stop(timeout=10) + master2.backupfile = master2.backupFS() + master2.start(timeout=10) + + # + # Here we have two instances master and consumer + # with replication working. Either coming from a backup recovery + # or from a fresh (re)init + # Time to return the topology + return TopologyMaster1Master2(master1, master2) + + + +def _bind_manager(server): + server.log.info("Bind as %s " % DN_DM) + server.simple_bind_s(DN_DM, PASSWORD) + +def _bind_normal(server): + server.log.info("Bind as %s " % BIND_DN) + server.simple_bind_s(BIND_DN, BIND_PW) + +def _header(topology, label): + topology.master1.log.info("\n\n###############################################") + topology.master1.log.info("#######") + topology.master1.log.info("####### %s" % label) + topology.master1.log.info("#######") + topology.master1.log.info("###############################################") + +def _status_entry_both_server(topology, name=None, desc=None, debug=True): + if not name: + return + topology.master1.log.info("\n\n######################### Tombstone on M1 ######################\n") + ent_m1 = _find_tombstone(topology.master1, SUFFIX, 'sn', name) + assert ent_m1 + + topology.master1.log.info("\n\n######################### Tombstone on M2 ######################\n") + ent_m2 = _find_tombstone(topology.master2, SUFFIX, 'sn', name) + assert ent_m2 + + topology.master1.log.info("\n\n######################### Description ######################\n%s\n" % desc) + topology.master1.log.info("M1 only\n") + for attr in ent_m1.getAttrs(): + + if not debug: + assert attr in ent_m2.getAttrs() + + if not attr in ent_m2.getAttrs(): + topology.master1.log.info(" %s" % attr) + for val in ent_m1.getValues(attr): + topology.master1.log.info(" %s" % val) + + topology.master1.log.info("M2 only\n") + for attr in ent_m2.getAttrs(): + + if not debug: + assert attr in ent_m1.getAttrs() + + if not attr in ent_m1.getAttrs(): + topology.master1.log.info(" %s" % attr) + for val in ent_m2.getValues(attr): + topology.master1.log.info(" %s" % val) + + topology.master1.log.info("M1 differs M2\n") + + if not debug: + assert ent_m1.dn == ent_m2.dn + + if ent_m1.dn != ent_m2.dn: + topology.master1.log.info(" M1[dn] = %s\n M2[dn] = %s" % (ent_m1.dn, ent_m2.dn)) + + for attr1 in ent_m1.getAttrs(): + if attr1 in ent_m2.getAttrs(): + for val1 in ent_m1.getValues(attr1): + found = False + for val2 in ent_m2.getValues(attr1): + if val1 == val2: + found = True + break + + if not debug: + assert found + + if not found: + topology.master1.log.info(" M1[%s] = %s" % (attr1, val1)) + + + for attr2 in ent_m2.getAttrs(): + if attr2 in ent_m1.getAttrs(): + for val2 in ent_m2.getValues(attr2): + found = False + for val1 in ent_m1.getValues(attr2): + if val2 == val1: + found = True + break + + if not debug: + assert found + + if not found: + topology.master1.log.info(" M2[%s] = %s" % (attr2, val2)) + +def _pause_RAs(topology): + topology.master1.log.info("\n\n######################### Pause RA M1<->M2 ######################\n") + ents = topology.master1.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master1.agreement.pause(ents[0].dn) + + ents = topology.master2.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master2.agreement.pause(ents[0].dn) + +def _resume_RAs(topology): + topology.master1.log.info("\n\n######################### resume RA M1<->M2 ######################\n") + ents = topology.master1.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master1.agreement.resume(ents[0].dn) + + ents = topology.master2.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master2.agreement.resume(ents[0].dn) + +def _find_tombstone(instance, base, attr, value): + # + # we can not use a filter with a (&(objeclass=nsTombstone)(sn=name)) because + # tombstone are not index in 'sn' so 'sn=name' will return NULL + # and even if tombstone are indexed for objectclass the '&' will set + # the candidate list to NULL + # + filt = '(objectclass=%s)' % REPLICA_OC_TOMBSTONE + ents = instance.search_s(base, ldap.SCOPE_SUBTREE, filt) + found = False + for ent in ents: + if ent.hasAttr(attr): + for val in ent.getValues(attr): + if val == value: + instance.log.debug("tombstone found: %r" % ent) + return ent + return None + + +def _delete_entry(instance, entry_dn, name): + instance.log.info("\n\n######################### DELETE %s (M1) ######################\n" % name) + + # delete the entry + instance.delete_s(entry_dn) + assert _find_tombstone(instance, SUFFIX, 'sn', name) != None + + + +def _mod_entry(instance, entry_dn, attr, value): + instance.log.info("\n\n######################### MOD %s (M2) ######################\n" % entry_dn) + mod = [(ldap.MOD_REPLACE, attr, value)] + instance.modify_s(entry_dn, mod) + +def _modrdn_entry(instance=None, entry_dn=None, new_rdn=None, del_old=0, new_superior=None): + assert instance != None + assert entry_dn != None + + if not new_rdn: + pattern = 'cn=(.*),(.*)' + rdnre = re.compile(pattern) + match = rdnre.match(entry_dn) + old_value = match.group(1) + new_rdn_val = "%s_modrdn" % old_value + new_rdn = "cn=%s" % new_rdn_val + + + instance.log.info("\n\n######################### MODRDN %s (M2) ######################\n" % new_rdn) + if new_superior: + instance.rename_s(entry_dn, new_rdn, newsuperior=new_superior, delold=del_old) + else: + instance.rename_s(entry_dn, new_rdn, delold=del_old) + +def _check_entry_exists(instance, entry_dn): + loop = 0 + while loop <= 10: + try: + ent = instance.getEntry(entry_dn, ldap.SCOPE_BASE, "(objectclass=*)") + break + except ldap.NO_SUCH_OBJECT: + time.sleep(1) + loop += 1 + assert loop <= 10 + +def _check_mod_received(instance, base, filt, attr, value): + instance.log.info("\n\n######################### Check MOD replicated on %s ######################\n" % instance.serverid) + loop = 0 + while loop <= 10: + ent = instance.getEntry(base, ldap.SCOPE_SUBTREE, filt) + if ent.hasAttr(attr) and ent.getValue(attr) == value: + break + time.sleep(1) + loop += 1 + assert loop <= 10 + +def _check_replication(topology, entry_dn): + # prepare the filter to retrieve the entry + filt = entry_dn.split(',')[0] + + topology.master1.log.info("\n######################### Check replicat M1->M2 ######################\n") + loop = 0 + while loop <= 10: + attr = 'description' + value = 'test_value_%d' % loop + mod = [(ldap.MOD_REPLACE, attr, value)] + topology.master1.modify_s(entry_dn, mod) + _check_mod_received(topology.master2, SUFFIX, filt, attr, value) + loop += 1 + + topology.master1.log.info("\n######################### Check replicat M2->M1 ######################\n") + loop = 0 + while loop <= 10: + attr = 'description' + value = 'test_value_%d' % loop + mod = [(ldap.MOD_REPLACE, attr, value)] + topology.master2.modify_s(entry_dn, mod) + _check_mod_received(topology.master1, SUFFIX, filt, attr, value) + loop += 1 + + +def test_ticket47787_init(topology): + """ + Creates + - a staging DIT + - a production DIT + - add accounts in staging DIT + + """ + + topology.master1.log.info("\n\n######################### INITIALIZATION ######################\n") + + # entry used to bind with + topology.master1.log.info("Add %s" % BIND_DN) + topology.master1.add_s(Entry((BIND_DN, { + 'objectclass': "top person".split(), + 'sn': BIND_CN, + 'cn': BIND_CN, + 'userpassword': BIND_PW}))) + + # DIT for staging + topology.master1.log.info("Add %s" % STAGING_DN) + topology.master1.add_s(Entry((STAGING_DN, { + 'objectclass': "top organizationalRole".split(), + 'cn': STAGING_CN, + 'description': "staging DIT"}))) + + # DIT for production + topology.master1.log.info("Add %s" % PRODUCTION_DN) + topology.master1.add_s(Entry((PRODUCTION_DN, { + 'objectclass': "top organizationalRole".split(), + 'cn': PRODUCTION_CN, + 'description': "production DIT"}))) + + + # enable replication error logging + mod = [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', '8192')] + topology.master1.modify_s(DN_CONFIG, mod) + topology.master2.modify_s(DN_CONFIG, mod) + + + + + + # add dummy entries in the staging DIT + for cpt in range(MAX_ACCOUNTS): + name = "%s%d" % (NEW_ACCOUNT, cpt) + topology.master1.add_s(Entry(("cn=%s,%s" % (name, STAGING_DN), { + 'objectclass': "top person".split(), + 'sn': name, + 'cn': name}))) + + +def test_ticket47787_2(topology): + ''' + Disable replication so that updates are not replicated + Delete an entry on M1. Modrdn it on M2 (chg rdn + delold=0 + same superior). + update a test entry on M2 + Reenable the RA. + checks that entry was deleted on M2 (with the modified RDN) + checks that test entry was replicated on M1 (replication M2->M1 not broken by modrdn) + ''' + + _header(topology, "test_ticket47787_2") + _bind_manager(topology.master1) + _bind_manager(topology.master2) + + #entry to test the replication is still working + name = "%s%d" % (NEW_ACCOUNT, MAX_ACCOUNTS -1) + test_rdn = "cn=%s" % (name) + testentry_dn = "%s,%s" % (test_rdn, STAGING_DN) + + name = "%s%d" % (NEW_ACCOUNT, MAX_ACCOUNTS - 2) + test2_rdn = "cn=%s" % (name) + testentry2_dn = "%s,%s" % (test2_rdn, STAGING_DN) + + # value of updates to test the replication both ways + attr = 'description' + value = 'test_ticket47787_2' + + # entry for the modrdn + name = "%s%d" % (NEW_ACCOUNT, 1) + rdn = "cn=%s" % (name) + entry_dn = "%s,%s" % (rdn, STAGING_DN) + + # created on M1, wait the entry exists on M2 + _check_entry_exists(topology.master2, entry_dn) + _check_entry_exists(topology.master2, testentry_dn) + + _pause_RAs(topology) + + # Delete 'entry_dn' on M1. + # dummy update is only have a first CSN before the DEL + # else the DEL will be in min_csn RUV and make diagnostic a bit more complex + _mod_entry(topology.master1, testentry2_dn, attr, 'dummy') + _delete_entry(topology.master1, entry_dn, name) + _mod_entry(topology.master1, testentry2_dn, attr, value) + + time.sleep(1) # important to have MOD.csn != DEL.csn + + # MOD 'entry_dn' on M1. + # dummy update is only have a first CSN before the MOD entry_dn + # else the DEL will be in min_csn RUV and make diagnostic a bit more complex + _mod_entry(topology.master2, testentry_dn, attr, 'dummy') + _mod_entry(topology.master2, entry_dn, attr, value) + _mod_entry(topology.master2, testentry_dn, attr, value) + + + _resume_RAs(topology) + + topology.master1.log.info("\n\n######################### Check DEL replicated on M2 ######################\n") + loop = 0 + while loop <= 10: + ent = _find_tombstone(topology.master2, SUFFIX, 'sn', name) + if ent: + break + time.sleep(1) + loop += 1 + assert loop <= 10 + assert ent + + # the following checks are not necessary + # as this bug is only for failing replicated MOD (entry_dn) on M1 + #_check_mod_received(topology.master1, SUFFIX, "(%s)" % (test_rdn), attr, value) + #_check_mod_received(topology.master2, SUFFIX, "(%s)" % (test2_rdn), attr, value) + # + #_check_replication(topology, testentry_dn) + + _status_entry_both_server(topology, name=name, desc="DEL M1 - MOD M2", debug=DEBUG_FLAG) + + topology.master1.log.info("\n\n######################### Check MOD replicated on M1 ######################\n") + loop = 0 + while loop <= 10: + ent = _find_tombstone(topology.master1, SUFFIX, 'sn', name) + if ent: + break + time.sleep(1) + loop += 1 + assert loop <= 10 + assert ent + assert ent.hasAttr(attr) + assert ent.getValue(attr) == value + + +def test_ticket47787_final(topology): + topology.master1.stop(timeout=10) + topology.master2.stop(timeout=10) + +def run_isolated(): + ''' + run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..) + To run isolated without py.test, you need to + - edit this file and comment '@pytest.fixture' line before 'topology' function. + - set the installation prefix + - run this program + ''' + global installation1_prefix + global installation2_prefix + installation1_prefix = None + installation2_prefix = None + + topo = topology(True) + topo.master1.log.info("\n\n######################### Ticket 47787 ######################\n") + test_ticket47787_init(topo) + + test_ticket47787_2(topo) + + test_ticket47787_final(topo) + + + + +if __name__ == '__main__': + run_isolated() + diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index f7ebac1..b6a889f 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -454,7 +454,7 @@ ldbm_back_modify( Slapi_PBlock *pb ) } if (!is_fixup_operation) { - if (slapi_entry_flag_is_set(e->ep_entry, SLAPI_ENTRY_FLAG_TOMBSTONE)) { + if (!repl_op && slapi_entry_flag_is_set(e->ep_entry, SLAPI_ENTRY_FLAG_TOMBSTONE)) { ldap_result_code = LDAP_UNWILLING_TO_PERFORM; ldap_result_message = "Operation not allowed on tombstone entry."; slapi_log_error(SLAPI_LOG_FATAL, "ldbm_back_modify",

1 0

Branch '389-ds-base-1.3.1' - dirsrvtests/tickets ldap/servers
by thierry bordaz 13 May '14

13 May '14

dirsrvtests/tickets/ticket47787_test.py | 630 +++++++++++++++++++++++++++++ ldap/servers/slapd/back-ldbm/ldbm_modify.c | 2 2 files changed, 631 insertions(+), 1 deletion(-) New commits: commit 1a9e1d813d408c41b57d57efa633ca92c149ba67 Author: Thierry bordaz (tbordaz) <tbordaz(a)redhat.com> Date: Fri Apr 25 15:07:08 2014 +0200 Ticket 47787 - A replicated MOD fails (Unwilling to perform) if it targets a tombstone Bug Description: A fix for https://fedorahosted.org/389/ticket/47396 prevents a MOD operation on a tombstone. A problem is that legacy version may allowed it or if a MOD entry and DEL entry occurs on different master, it may happen that a consumer receive a MOD on a tombstone. The server return 'Unwilling to perform' that may break replication. Fix Description: Allows a MOD on tombstone, only if it comes from a replication session https://fedorahosted.org/389/ticket/47787 Reviewed by: Rich Megginson (thanks) Platforms tested: F17, F19 Flag Day: no Doc impact: no diff --git a/dirsrvtests/tickets/ticket47787_test.py b/dirsrvtests/tickets/ticket47787_test.py new file mode 100644 index 0000000..d9d3ca9 --- /dev/null +++ b/dirsrvtests/tickets/ticket47787_test.py @@ -0,0 +1,630 @@ +''' +Created on April 14, 2014 + +@author: tbordaz +''' +import os +import sys +import time +import ldap +import logging +import socket +import time +import logging +import pytest +import re +from lib389 import DirSrv, Entry, tools, NoSuchEntryError +from lib389.tools import DirSrvTools +from lib389._constants import * +from lib389.properties import * +from constants import * +from lib389._constants import REPLICAROLE_MASTER + +logging.getLogger(__name__).setLevel(logging.DEBUG) +log = logging.getLogger(__name__) + +# +# important part. We can deploy Master1 and Master2 on different versions +# +installation1_prefix = None +installation2_prefix = None + +# set this flag to False so that it will assert on failure _status_entry_both_server +DEBUG_FLAG = False + +TEST_REPL_DN = "cn=test_repl, %s" % SUFFIX + +STAGING_CN = "staged user" +PRODUCTION_CN = "accounts" +EXCEPT_CN = "excepts" + +STAGING_DN = "cn=%s,%s" % (STAGING_CN, SUFFIX) +PRODUCTION_DN = "cn=%s,%s" % (PRODUCTION_CN, SUFFIX) +PROD_EXCEPT_DN = "cn=%s,%s" % (EXCEPT_CN, PRODUCTION_DN) + +STAGING_PATTERN = "cn=%s*,%s" % (STAGING_CN[:2], SUFFIX) +PRODUCTION_PATTERN = "cn=%s*,%s" % (PRODUCTION_CN[:2], SUFFIX) +BAD_STAGING_PATTERN = "cn=bad*,%s" % (SUFFIX) +BAD_PRODUCTION_PATTERN = "cn=bad*,%s" % (SUFFIX) + +BIND_CN = "bind_entry" +BIND_DN = "cn=%s,%s" % (BIND_CN, SUFFIX) +BIND_PW = "password" + +NEW_ACCOUNT = "new_account" +MAX_ACCOUNTS = 20 + +CONFIG_MODDN_ACI_ATTR = "nsslapd-moddn-aci" + +class TopologyMaster1Master2(object): + def __init__(self, master1, master2): + master1.open() + self.master1 = master1 + + master2.open() + self.master2 = master2 + + +(a)pytest.fixture(scope="module") +def topology(request): + ''' + This fixture is used to create a replicated topology for the 'module'. + The replicated topology is MASTER1 <-> Master2. + At the beginning, It may exists a master2 instance and/or a master2 instance. + It may also exists a backup for the master1 and/or the master2. + + Principle: + If master1 instance exists: + restart it + If master2 instance exists: + restart it + If backup of master1 AND backup of master2 exists: + create or rebind to master1 + create or rebind to master2 + + restore master1 from backup + restore master2 from backup + else: + Cleanup everything + remove instances + remove backups + Create instances + Initialize replication + Create backups + ''' + global installation1_prefix + global installation2_prefix + + # allocate master1 on a given deployement + master1 = DirSrv(verbose=False) + if installation1_prefix: + args_instance[SER_DEPLOYED_DIR] = installation1_prefix + + # Args for the master1 instance + args_instance[SER_HOST] = HOST_MASTER_1 + args_instance[SER_PORT] = PORT_MASTER_1 + args_instance[SER_SERVERID_PROP] = SERVERID_MASTER_1 + args_master = args_instance.copy() + master1.allocate(args_master) + + # allocate master1 on a given deployement + master2 = DirSrv(verbose=False) + if installation2_prefix: + args_instance[SER_DEPLOYED_DIR] = installation2_prefix + + # Args for the consumer instance + args_instance[SER_HOST] = HOST_MASTER_2 + args_instance[SER_PORT] = PORT_MASTER_2 + args_instance[SER_SERVERID_PROP] = SERVERID_MASTER_2 + args_master = args_instance.copy() + master2.allocate(args_master) + + + # Get the status of the backups + backup_master1 = master1.checkBackupFS() + backup_master2 = master2.checkBackupFS() + + # Get the status of the instance and restart it if it exists + instance_master1 = master1.exists() + if instance_master1: + master1.stop(timeout=10) + master1.start(timeout=10) + + instance_master2 = master2.exists() + if instance_master2: + master2.stop(timeout=10) + master2.start(timeout=10) + + if backup_master1 and backup_master2: + # The backups exist, assuming they are correct + # we just re-init the instances with them + if not instance_master1: + master1.create() + # Used to retrieve configuration information (dbdir, confdir...) + master1.open() + + if not instance_master2: + master2.create() + # Used to retrieve configuration information (dbdir, confdir...) + master2.open() + + # restore master1 from backup + master1.stop(timeout=10) + master1.restoreFS(backup_master1) + master1.start(timeout=10) + + # restore master2 from backup + master2.stop(timeout=10) + master2.restoreFS(backup_master2) + master2.start(timeout=10) + else: + # We should be here only in two conditions + # - This is the first time a test involve master-consumer + # so we need to create everything + # - Something weird happened (instance/backup destroyed) + # so we discard everything and recreate all + + # Remove all the backups. So even if we have a specific backup file + # (e.g backup_master) we clear all backups that an instance my have created + if backup_master1: + master1.clearBackupFS() + if backup_master2: + master2.clearBackupFS() + + # Remove all the instances + if instance_master1: + master1.delete() + if instance_master2: + master2.delete() + + # Create the instances + master1.create() + master1.open() + master2.create() + master2.open() + + # + # Now prepare the Master-Consumer topology + # + # First Enable replication + master1.replica.enableReplication(suffix=SUFFIX, role=REPLICAROLE_MASTER, replicaId=REPLICAID_MASTER_1) + master2.replica.enableReplication(suffix=SUFFIX, role=REPLICAROLE_MASTER, replicaId=REPLICAID_MASTER_2) + + # Initialize the supplier->consumer + + properties = {RA_NAME: r'meTo_$host:$port', + RA_BINDDN: defaultProperties[REPLICATION_BIND_DN], + RA_BINDPW: defaultProperties[REPLICATION_BIND_PW], + RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD], + RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + repl_agreement = master1.agreement.create(suffix=SUFFIX, host=master2.host, port=master2.port, properties=properties) + + if not repl_agreement: + log.fatal("Fail to create a replica agreement") + sys.exit(1) + + log.debug("%s created" % repl_agreement) + + properties = {RA_NAME: r'meTo_$host:$port', + RA_BINDDN: defaultProperties[REPLICATION_BIND_DN], + RA_BINDPW: defaultProperties[REPLICATION_BIND_PW], + RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD], + RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + master2.agreement.create(suffix=SUFFIX, host=master1.host, port=master1.port, properties=properties) + + master1.agreement.init(SUFFIX, HOST_MASTER_2, PORT_MASTER_2) + master1.waitForReplInit(repl_agreement) + + # Check replication is working fine + master1.add_s(Entry((TEST_REPL_DN, { + 'objectclass': "top person".split(), + 'sn': 'test_repl', + 'cn': 'test_repl'}))) + loop = 0 + while loop <= 10: + try: + ent = master2.getEntry(TEST_REPL_DN, ldap.SCOPE_BASE, "(objectclass=*)") + break + except ldap.NO_SUCH_OBJECT: + time.sleep(1) + loop += 1 + + # Time to create the backups + master1.stop(timeout=10) + master1.backupfile = master1.backupFS() + master1.start(timeout=10) + + master2.stop(timeout=10) + master2.backupfile = master2.backupFS() + master2.start(timeout=10) + + # + # Here we have two instances master and consumer + # with replication working. Either coming from a backup recovery + # or from a fresh (re)init + # Time to return the topology + return TopologyMaster1Master2(master1, master2) + + + +def _bind_manager(server): + server.log.info("Bind as %s " % DN_DM) + server.simple_bind_s(DN_DM, PASSWORD) + +def _bind_normal(server): + server.log.info("Bind as %s " % BIND_DN) + server.simple_bind_s(BIND_DN, BIND_PW) + +def _header(topology, label): + topology.master1.log.info("\n\n###############################################") + topology.master1.log.info("#######") + topology.master1.log.info("####### %s" % label) + topology.master1.log.info("#######") + topology.master1.log.info("###############################################") + +def _status_entry_both_server(topology, name=None, desc=None, debug=True): + if not name: + return + topology.master1.log.info("\n\n######################### Tombstone on M1 ######################\n") + ent_m1 = _find_tombstone(topology.master1, SUFFIX, 'sn', name) + assert ent_m1 + + topology.master1.log.info("\n\n######################### Tombstone on M2 ######################\n") + ent_m2 = _find_tombstone(topology.master2, SUFFIX, 'sn', name) + assert ent_m2 + + topology.master1.log.info("\n\n######################### Description ######################\n%s\n" % desc) + topology.master1.log.info("M1 only\n") + for attr in ent_m1.getAttrs(): + + if not debug: + assert attr in ent_m2.getAttrs() + + if not attr in ent_m2.getAttrs(): + topology.master1.log.info(" %s" % attr) + for val in ent_m1.getValues(attr): + topology.master1.log.info(" %s" % val) + + topology.master1.log.info("M2 only\n") + for attr in ent_m2.getAttrs(): + + if not debug: + assert attr in ent_m1.getAttrs() + + if not attr in ent_m1.getAttrs(): + topology.master1.log.info(" %s" % attr) + for val in ent_m2.getValues(attr): + topology.master1.log.info(" %s" % val) + + topology.master1.log.info("M1 differs M2\n") + + if not debug: + assert ent_m1.dn == ent_m2.dn + + if ent_m1.dn != ent_m2.dn: + topology.master1.log.info(" M1[dn] = %s\n M2[dn] = %s" % (ent_m1.dn, ent_m2.dn)) + + for attr1 in ent_m1.getAttrs(): + if attr1 in ent_m2.getAttrs(): + for val1 in ent_m1.getValues(attr1): + found = False + for val2 in ent_m2.getValues(attr1): + if val1 == val2: + found = True + break + + if not debug: + assert found + + if not found: + topology.master1.log.info(" M1[%s] = %s" % (attr1, val1)) + + + for attr2 in ent_m2.getAttrs(): + if attr2 in ent_m1.getAttrs(): + for val2 in ent_m2.getValues(attr2): + found = False + for val1 in ent_m1.getValues(attr2): + if val2 == val1: + found = True + break + + if not debug: + assert found + + if not found: + topology.master1.log.info(" M2[%s] = %s" % (attr2, val2)) + +def _pause_RAs(topology): + topology.master1.log.info("\n\n######################### Pause RA M1<->M2 ######################\n") + ents = topology.master1.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master1.agreement.pause(ents[0].dn) + + ents = topology.master2.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master2.agreement.pause(ents[0].dn) + +def _resume_RAs(topology): + topology.master1.log.info("\n\n######################### resume RA M1<->M2 ######################\n") + ents = topology.master1.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master1.agreement.resume(ents[0].dn) + + ents = topology.master2.agreement.list(suffix=SUFFIX) + assert len(ents) == 1 + topology.master2.agreement.resume(ents[0].dn) + +def _find_tombstone(instance, base, attr, value): + # + # we can not use a filter with a (&(objeclass=nsTombstone)(sn=name)) because + # tombstone are not index in 'sn' so 'sn=name' will return NULL + # and even if tombstone are indexed for objectclass the '&' will set + # the candidate list to NULL + # + filt = '(objectclass=%s)' % REPLICA_OC_TOMBSTONE + ents = instance.search_s(base, ldap.SCOPE_SUBTREE, filt) + found = False + for ent in ents: + if ent.hasAttr(attr): + for val in ent.getValues(attr): + if val == value: + instance.log.debug("tombstone found: %r" % ent) + return ent + return None + + +def _delete_entry(instance, entry_dn, name): + instance.log.info("\n\n######################### DELETE %s (M1) ######################\n" % name) + + # delete the entry + instance.delete_s(entry_dn) + assert _find_tombstone(instance, SUFFIX, 'sn', name) != None + + + +def _mod_entry(instance, entry_dn, attr, value): + instance.log.info("\n\n######################### MOD %s (M2) ######################\n" % entry_dn) + mod = [(ldap.MOD_REPLACE, attr, value)] + instance.modify_s(entry_dn, mod) + +def _modrdn_entry(instance=None, entry_dn=None, new_rdn=None, del_old=0, new_superior=None): + assert instance != None + assert entry_dn != None + + if not new_rdn: + pattern = 'cn=(.*),(.*)' + rdnre = re.compile(pattern) + match = rdnre.match(entry_dn) + old_value = match.group(1) + new_rdn_val = "%s_modrdn" % old_value + new_rdn = "cn=%s" % new_rdn_val + + + instance.log.info("\n\n######################### MODRDN %s (M2) ######################\n" % new_rdn) + if new_superior: + instance.rename_s(entry_dn, new_rdn, newsuperior=new_superior, delold=del_old) + else: + instance.rename_s(entry_dn, new_rdn, delold=del_old) + +def _check_entry_exists(instance, entry_dn): + loop = 0 + while loop <= 10: + try: + ent = instance.getEntry(entry_dn, ldap.SCOPE_BASE, "(objectclass=*)") + break + except ldap.NO_SUCH_OBJECT: + time.sleep(1) + loop += 1 + assert loop <= 10 + +def _check_mod_received(instance, base, filt, attr, value): + instance.log.info("\n\n######################### Check MOD replicated on %s ######################\n" % instance.serverid) + loop = 0 + while loop <= 10: + ent = instance.getEntry(base, ldap.SCOPE_SUBTREE, filt) + if ent.hasAttr(attr) and ent.getValue(attr) == value: + break + time.sleep(1) + loop += 1 + assert loop <= 10 + +def _check_replication(topology, entry_dn): + # prepare the filter to retrieve the entry + filt = entry_dn.split(',')[0] + + topology.master1.log.info("\n######################### Check replicat M1->M2 ######################\n") + loop = 0 + while loop <= 10: + attr = 'description' + value = 'test_value_%d' % loop + mod = [(ldap.MOD_REPLACE, attr, value)] + topology.master1.modify_s(entry_dn, mod) + _check_mod_received(topology.master2, SUFFIX, filt, attr, value) + loop += 1 + + topology.master1.log.info("\n######################### Check replicat M2->M1 ######################\n") + loop = 0 + while loop <= 10: + attr = 'description' + value = 'test_value_%d' % loop + mod = [(ldap.MOD_REPLACE, attr, value)] + topology.master2.modify_s(entry_dn, mod) + _check_mod_received(topology.master1, SUFFIX, filt, attr, value) + loop += 1 + + +def test_ticket47787_init(topology): + """ + Creates + - a staging DIT + - a production DIT + - add accounts in staging DIT + + """ + + topology.master1.log.info("\n\n######################### INITIALIZATION ######################\n") + + # entry used to bind with + topology.master1.log.info("Add %s" % BIND_DN) + topology.master1.add_s(Entry((BIND_DN, { + 'objectclass': "top person".split(), + 'sn': BIND_CN, + 'cn': BIND_CN, + 'userpassword': BIND_PW}))) + + # DIT for staging + topology.master1.log.info("Add %s" % STAGING_DN) + topology.master1.add_s(Entry((STAGING_DN, { + 'objectclass': "top organizationalRole".split(), + 'cn': STAGING_CN, + 'description': "staging DIT"}))) + + # DIT for production + topology.master1.log.info("Add %s" % PRODUCTION_DN) + topology.master1.add_s(Entry((PRODUCTION_DN, { + 'objectclass': "top organizationalRole".split(), + 'cn': PRODUCTION_CN, + 'description': "production DIT"}))) + + + # enable replication error logging + mod = [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', '8192')] + topology.master1.modify_s(DN_CONFIG, mod) + topology.master2.modify_s(DN_CONFIG, mod) + + + + + + # add dummy entries in the staging DIT + for cpt in range(MAX_ACCOUNTS): + name = "%s%d" % (NEW_ACCOUNT, cpt) + topology.master1.add_s(Entry(("cn=%s,%s" % (name, STAGING_DN), { + 'objectclass': "top person".split(), + 'sn': name, + 'cn': name}))) + + +def test_ticket47787_2(topology): + ''' + Disable replication so that updates are not replicated + Delete an entry on M1. Modrdn it on M2 (chg rdn + delold=0 + same superior). + update a test entry on M2 + Reenable the RA. + checks that entry was deleted on M2 (with the modified RDN) + checks that test entry was replicated on M1 (replication M2->M1 not broken by modrdn) + ''' + + _header(topology, "test_ticket47787_2") + _bind_manager(topology.master1) + _bind_manager(topology.master2) + + #entry to test the replication is still working + name = "%s%d" % (NEW_ACCOUNT, MAX_ACCOUNTS -1) + test_rdn = "cn=%s" % (name) + testentry_dn = "%s,%s" % (test_rdn, STAGING_DN) + + name = "%s%d" % (NEW_ACCOUNT, MAX_ACCOUNTS - 2) + test2_rdn = "cn=%s" % (name) + testentry2_dn = "%s,%s" % (test2_rdn, STAGING_DN) + + # value of updates to test the replication both ways + attr = 'description' + value = 'test_ticket47787_2' + + # entry for the modrdn + name = "%s%d" % (NEW_ACCOUNT, 1) + rdn = "cn=%s" % (name) + entry_dn = "%s,%s" % (rdn, STAGING_DN) + + # created on M1, wait the entry exists on M2 + _check_entry_exists(topology.master2, entry_dn) + _check_entry_exists(topology.master2, testentry_dn) + + _pause_RAs(topology) + + # Delete 'entry_dn' on M1. + # dummy update is only have a first CSN before the DEL + # else the DEL will be in min_csn RUV and make diagnostic a bit more complex + _mod_entry(topology.master1, testentry2_dn, attr, 'dummy') + _delete_entry(topology.master1, entry_dn, name) + _mod_entry(topology.master1, testentry2_dn, attr, value) + + time.sleep(1) # important to have MOD.csn != DEL.csn + + # MOD 'entry_dn' on M1. + # dummy update is only have a first CSN before the MOD entry_dn + # else the DEL will be in min_csn RUV and make diagnostic a bit more complex + _mod_entry(topology.master2, testentry_dn, attr, 'dummy') + _mod_entry(topology.master2, entry_dn, attr, value) + _mod_entry(topology.master2, testentry_dn, attr, value) + + + _resume_RAs(topology) + + topology.master1.log.info("\n\n######################### Check DEL replicated on M2 ######################\n") + loop = 0 + while loop <= 10: + ent = _find_tombstone(topology.master2, SUFFIX, 'sn', name) + if ent: + break + time.sleep(1) + loop += 1 + assert loop <= 10 + assert ent + + # the following checks are not necessary + # as this bug is only for failing replicated MOD (entry_dn) on M1 + #_check_mod_received(topology.master1, SUFFIX, "(%s)" % (test_rdn), attr, value) + #_check_mod_received(topology.master2, SUFFIX, "(%s)" % (test2_rdn), attr, value) + # + #_check_replication(topology, testentry_dn) + + _status_entry_both_server(topology, name=name, desc="DEL M1 - MOD M2", debug=DEBUG_FLAG) + + topology.master1.log.info("\n\n######################### Check MOD replicated on M1 ######################\n") + loop = 0 + while loop <= 10: + ent = _find_tombstone(topology.master1, SUFFIX, 'sn', name) + if ent: + break + time.sleep(1) + loop += 1 + assert loop <= 10 + assert ent + assert ent.hasAttr(attr) + assert ent.getValue(attr) == value + + +def test_ticket47787_final(topology): + topology.master1.stop(timeout=10) + topology.master2.stop(timeout=10) + +def run_isolated(): + ''' + run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..) + To run isolated without py.test, you need to + - edit this file and comment '@pytest.fixture' line before 'topology' function. + - set the installation prefix + - run this program + ''' + global installation1_prefix + global installation2_prefix + installation1_prefix = None + installation2_prefix = None + + topo = topology(True) + topo.master1.log.info("\n\n######################### Ticket 47787 ######################\n") + test_ticket47787_init(topo) + + test_ticket47787_2(topo) + + test_ticket47787_final(topo) + + + + +if __name__ == '__main__': + run_isolated() + diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index ac84ccb..31e2f66 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c @@ -529,7 +529,7 @@ ldbm_back_modify( Slapi_PBlock *pb ) if ( !is_fixup_operation ) { - if (slapi_entry_flag_is_set(e->ep_entry, SLAPI_ENTRY_FLAG_TOMBSTONE) ) { + if (!repl_op && slapi_entry_flag_is_set(e->ep_entry, SLAPI_ENTRY_FLAG_TOMBSTONE) ) { ldap_result_code = LDAP_UNWILLING_TO_PERFORM; ldap_result_message = "Operation not allowed on tombstone entry."; slapi_log_error(SLAPI_LOG_FATAL, "ldbm_back_modify",

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

389-commits May 2014