#48964 cleanallruv changelog cleaning incorrectly impacts all backends
Closed: Fixed None Opened 3 years ago by mreynolds.

At the end of the cleanAllRUV task the changelog is purged of entries that contain the invalid rid, but it "cleans" all the backend changelogs. It should only clean the specific backend changelog specified in the clean task.


f2ecfcc..fda0043 master -> master
commit fda0043
Author: Mark Reynolds mreynolds@redhat.com
Date: Tue Aug 23 12:06:30 2016 -0400

c7fb671..792aa66 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 792aa66

2191728..f4df06b 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit f4df06b

855c34e..0d5afb2 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 0d5afb2

81c6e66..a38d76d master -> master
commit a38d76d
Author: Mark Reynolds mreynolds@redhat.com
Date: Thu Dec 22 14:38:27 2016 -0500

460c578..4053587 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit 4053587

262f095..7b39835 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 7b39835

f850128..41c0a49 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit 41c0a49

Hi Mark,

Covscan reported this error in the patch. Since replica is asserted as non-NULL, could you please remove the NULL check at the line 1471? Thanks.
{{{
1439 PR_ASSERT (replica);
}}}

{{{
1. Defect type: REVERSE_INULL ΒΆ
1. 389-ds-base-1.3.5.10/ldap/servers/plugins/replication/repl5_replica_config.c:1442: deref_ptr_in_call: Dereferencing pointer "replica".
2. 389-ds-base-1.3.5.10/ldap/servers/plugins/replication/repl5_replica.c:781:2: deref_parm: Directly dereferencing parameter "r".
3. 389-ds-base-1.3.5.10/ldap/servers/plugins/replication/repl5_replica_config.c:1471: check_after_deref: Null-checking "replica" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

1469| * Now purge the changelog. The purging thread will free the purge_data

1470| */

1471|-> if (replica){

1472| purge_data = (cleanruv_purge_data*)slapi_ch_calloc(1, sizeof(cleanruv_purge_data));

1473| purge_data->cleaned_rid = rid;

}}}

Hi Mark,

The patch looks good, just a minor comment.
free_purge_data does free ''purge_data->replName'' that is the replica name (replica->repl_name).
I think it should not because the replica structure should survive from '_cl5DoPurging'.
Free of ''replGen'' is valid because it was a duplicated string.

Replying to [comment:11 tbordaz]:

Hi Mark,

The patch looks good, just a minor comment.
free_purge_data does free ''purge_data->replName'' that is the replica name (replica->repl_name).
I think it should not because the replica structure should survive from '_cl5DoPurging'.
Free of ''replGen'' is valid because it was a duplicated string.

You're right, I incorrectly thought replica_get_name() made a strdup of the name. New patch coming soon.

Thanks Mark, the patch is looking good to me --> ACK

b032f71..017469a master -> master
commit 017469a
Author: Mark Reynolds mreynolds@redhat.com
Date: Wed Jan 4 09:41:38 2017 -0500

4053587..0929992 389-ds-base-1.3.5 -> 389-ds-base-1.3.5
commit 0929992

7b39835..56a24df 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit 56a24df

66f0aa8..d4d1072 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit d4d1072

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.2.11.33

2 years ago

Login to comment on this ticket.

Metadata