From f4a3304477e6d9a8488b111ebec4223d2fc378cf Mon Sep 17 00:00:00 2001 From: Hallvard Furuseth Date: Fri, 31 Dec 2010 17:55:36 +0000 Subject: [PATCH] ITS#6758 Use rs_*() to manage SlapReply entries. Some noop changes + fixes entry leaks and REP_ENTRY_MODIFIABLE flag leaks. --- contrib/slapd-modules/cloak/cloak.c | 14 ++------------ contrib/slapd-modules/usn/usn.c | 5 +---- servers/slapd/overlays/collect.c | 14 +------------- servers/slapd/overlays/pcache.c | 6 +----- servers/slapd/overlays/syncprov.c | 12 +----------- servers/slapd/overlays/translucent.c | 27 +++------------------------ servers/slapd/overlays/valsort.c | 15 +-------------- servers/slapd/result.c | 11 ++++------- servers/slapd/slapi/slapi_pblock.c | 10 ++-------- 9 files changed, 16 insertions(+), 98 deletions(-) diff --git a/contrib/slapd-modules/cloak/cloak.c b/contrib/slapd-modules/cloak/cloak.c index 03c9c09f91..b6746e9ce8 100644 --- a/contrib/slapd-modules/cloak/cloak.c +++ b/contrib/slapd-modules/cloak/cloak.c @@ -221,10 +221,8 @@ cloak_search_cb( Operation *op, SlapReply *rs ) /* * We are now committed to cloak an attribute. */ - if ( rs->sr_flags & REP_ENTRY_MODIFIABLE ) - me = e; - else - me = entry_dup( e ); + rs_ensure_entry_modifiable( op, rs, (slap_overinst *) op->o_bd->bd_info ); + me = rs->sr_entry; for ( ci = (cloak_info_t *)sc->sc_private; ci; ci = ci->ci_next ) { Attribute *a; @@ -251,14 +249,6 @@ cloak_search_cb( Operation *op, SlapReply *rs ) } - if ( me != e ) { - if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) - entry_free( e ); - - rs->sr_entry = me; - rs->sr_flags |= REP_ENTRY_MODIFIABLE | REP_ENTRY_MUSTBEFREED; - } - return ( SLAP_CB_CONTINUE ); } diff --git a/contrib/slapd-modules/usn/usn.c b/contrib/slapd-modules/usn/usn.c index bbf09c5cbb..cb622eb780 100644 --- a/contrib/slapd-modules/usn/usn.c +++ b/contrib/slapd-modules/usn/usn.c @@ -135,10 +135,7 @@ usn_operational( } if ( !ap ) { - if ( !rs->sr_flags & REP_ENTRY_MODIFIABLE ) { - rs->sr_entry = entry_dup( rs->sr_entry ); - rs->sr_flags |= - REP_ENTRY_MODIFIABLE|REP_ENTRY_MUSTBEFREED; + if ( rs_ensure_entry_modifiable( op,rs, on )) { a = attr_find( rs->sr_entry->e_attrs, ad_usnChanged ); } diff --git a/servers/slapd/overlays/collect.c b/servers/slapd/overlays/collect.c index 48fec67cb9..6c9a265111 100644 --- a/servers/slapd/overlays/collect.c +++ b/servers/slapd/overlays/collect.c @@ -386,19 +386,7 @@ collect_response( Operation *op, SlapReply *rs ) * don't modify it directly. Make a copy and * work with that instead. */ - if ( !( rs->sr_flags & REP_ENTRY_MODIFIABLE ) ) { - Entry *e; - - e = entry_dup( rs->sr_entry ); - if ( rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - overlay_entry_release_ov( op, rs->sr_entry, 0, on ); - rs->sr_flags &= ~REP_ENTRY_MUSTRELEASE; - } else if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - entry_free( rs->sr_entry ); - } - rs->sr_entry = e; - rs->sr_flags |= REP_ENTRY_MODIFIABLE|REP_ENTRY_MUSTBEFREED; - } + rs_ensure_entry_modifiable( op, rs, on ); /* Loop for each attribute in this collectinfo */ for(idx=0; idxci_ad_num; idx++) { diff --git a/servers/slapd/overlays/pcache.c b/servers/slapd/overlays/pcache.c index e584e435d9..c5f2100f96 100644 --- a/servers/slapd/overlays/pcache.c +++ b/servers/slapd/overlays/pcache.c @@ -2030,11 +2030,7 @@ fetch_queryId_cb( Operation *op, SlapReply *rs ) } /* clear entry if required */ - if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - entry_free( rs->sr_entry ); - rs->sr_entry = NULL; - rs->sr_flags ^= REP_ENTRY_MUSTBEFREED; - } + rs_flush_entry( op, rs, (slap_overinst *) op->o_bd->bd_info ); return rc; } diff --git a/servers/slapd/overlays/syncprov.c b/servers/slapd/overlays/syncprov.c index 6f8fb7f338..3834dd5a6d 100644 --- a/servers/slapd/overlays/syncprov.c +++ b/servers/slapd/overlays/syncprov.c @@ -2740,17 +2740,7 @@ syncprov_operational( } if ( !ap ) { - if ( !(rs->sr_flags & REP_ENTRY_MODIFIABLE) ) { - Entry *e = entry_dup( rs->sr_entry ); - if ( rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - overlay_entry_release_ov( op, rs->sr_entry, 0, on ); - rs->sr_flags ^= REP_ENTRY_MUSTRELEASE; - } else if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - entry_free( rs->sr_entry ); - } - rs->sr_entry = e; - rs->sr_flags |= - REP_ENTRY_MODIFIABLE|REP_ENTRY_MUSTBEFREED; + if ( rs_ensure_entry_modifiable( op, rs, on )) { a = attr_find( rs->sr_entry->e_attrs, slap_schema.si_ad_contextCSN ); } diff --git a/servers/slapd/overlays/translucent.c b/servers/slapd/overlays/translucent.c index 2b07607898..9ba368da98 100644 --- a/servers/slapd/overlays/translucent.c +++ b/servers/slapd/overlays/translucent.c @@ -815,14 +815,7 @@ static int translucent_search_cb(Operation *op, SlapReply *rs) { if ( tc->step & USE_LIST ) { re = tavl_delete( &tc->list, le, entry_dn_cmp ); if ( re ) { - if ( rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - rs->sr_flags ^= REP_ENTRY_MUSTRELEASE; - overlay_entry_release_ov( op, rs->sr_entry, 0, on ); - } - if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - rs->sr_flags ^= REP_ENTRY_MUSTBEFREED; - entry_free( rs->sr_entry ); - } + rs_flush_entry( op, rs, on ); rc = test_filter( op, re, tc->orig ); if ( rc == LDAP_COMPARE_TRUE ) { rs->sr_flags |= REP_ENTRY_MUSTBEFREED; @@ -855,14 +848,7 @@ static int translucent_search_cb(Operation *op, SlapReply *rs) { rc = overlay_entry_get_ov(op, &rs->sr_entry->e_nname, NULL, NULL, 0, &le, on); if ( rc == LDAP_SUCCESS && le ) { re = entry_dup( rs->sr_entry ); - if ( rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - rs->sr_flags ^= REP_ENTRY_MUSTRELEASE; - overlay_entry_release_ov( op, rs->sr_entry, 0, on ); - } - if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - rs->sr_flags ^= REP_ENTRY_MUSTBEFREED; - entry_free( rs->sr_entry ); - } + rs_flush_entry( op, rs, on ); } else { le = NULL; } @@ -902,14 +888,7 @@ static int translucent_search_cb(Operation *op, SlapReply *rs) { } /* Dispose of local entry */ if ( tc->step & LCL_SIDE ) { - if ( rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - rs->sr_flags ^= REP_ENTRY_MUSTRELEASE; - overlay_entry_release_ov( op, rs->sr_entry, 0, on ); - } - if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - rs->sr_flags ^= REP_ENTRY_MUSTBEFREED; - entry_free( rs->sr_entry ); - } + rs_flush_entry(op, rs, on); } else { overlay_entry_release_ov(op, le, 0, on); } diff --git a/servers/slapd/overlays/valsort.c b/servers/slapd/overlays/valsort.c index e516e46269..a64f189242 100644 --- a/servers/slapd/overlays/valsort.c +++ b/servers/slapd/overlays/valsort.c @@ -297,20 +297,7 @@ valsort_response( Operation *op, SlapReply *rs ) a = attr_find( rs->sr_entry->e_attrs, vi->vi_ad ); if ( !a ) continue; - if (( rs->sr_flags & ( REP_ENTRY_MODIFIABLE|REP_ENTRY_MUSTBEFREED ) ) != - ( REP_ENTRY_MODIFIABLE|REP_ENTRY_MUSTBEFREED ) ) - { - Entry *e; - - e = entry_dup( rs->sr_entry ); - if ( rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - overlay_entry_release_ov( op, rs->sr_entry, 0, on ); - rs->sr_flags &= ~REP_ENTRY_MUSTRELEASE; - } else if ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - entry_free( rs->sr_entry ); - } - rs->sr_entry = e; - rs->sr_flags |= REP_ENTRY_MODIFIABLE|REP_ENTRY_MUSTBEFREED; + if ( rs_ensure_entry_modifiable( op, rs, on )) { a = attr_find( rs->sr_entry->e_attrs, vi->vi_ad ); } diff --git a/servers/slapd/result.c b/servers/slapd/result.c index 1f6a2f3fc8..e70ac54c08 100644 --- a/servers/slapd/result.c +++ b/servers/slapd/result.c @@ -1407,13 +1407,10 @@ error_return:; * should set it back so that the cleanup functions know * what they're doing. */ - if ( op->o_tag == LDAP_REQ_SEARCH && rs->sr_type == REP_SEARCH - && rs->sr_entry - && ( rs->sr_flags & REP_ENTRY_MUSTBEFREED ) ) - { - entry_free( rs->sr_entry ); - rs->sr_entry = NULL; - rs->sr_flags &= ~REP_ENTRY_MUSTBEFREED; + if ( op->o_tag == LDAP_REQ_SEARCH && rs->sr_type == REP_SEARCH ) { + rs_flush_entry( op, rs, NULL ); + } else { + RS_ASSERT( (rs->sr_flags & REP_ENTRY_MASK) == 0 ); } if ( rs->sr_flags & REP_CTRLS_MUSTBEFREED ) { diff --git a/servers/slapd/slapi/slapi_pblock.c b/servers/slapd/slapi/slapi_pblock.c index bbc91d4a4e..31b0dc3566 100644 --- a/servers/slapd/slapi/slapi_pblock.c +++ b/servers/slapd/slapi/slapi_pblock.c @@ -1176,13 +1176,8 @@ pblock_set( Slapi_PBlock *pb, int param, void *value ) break; case SLAPI_SEARCH_RESULT_ENTRY: PBLOCK_ASSERT_OP( pb, 0 ); - if ( pb->pb_rs->sr_flags & REP_ENTRY_MUSTBEFREED ) { - entry_free( pb->pb_rs->sr_entry ); - } else if ( pb->pb_rs->sr_flags & REP_ENTRY_MUSTRELEASE ) { - be_entry_release_r( pb->pb_op, pb->pb_rs->sr_entry ); - pb->pb_rs->sr_flags ^= REP_ENTRY_MUSTRELEASE; - } - pb->pb_rs->sr_entry = (Slapi_Entry *)value; + rs_replace_entry( pb->pb_op, pb->pb_rs, NULL, (Slapi_Entry *)value ); + /* TODO: Should REP_ENTRY_MODIFIABLE be set? */ pb->pb_rs->sr_flags |= REP_ENTRY_MUSTBEFREED; break; case SLAPI_BIND_RET_SASLCREDS: @@ -1429,4 +1424,3 @@ slapi_int_pblock_get_next( Slapi_PBlock **pb ) } #endif /* LDAP_SLAPI */ - -- 2.39.5