ldap/servers/slapd/plugin.c | 175 ++++++++++++++++++----------------------
ldap/servers/slapd/proto-slap.h | 5 -
3 files changed, 96 insertions(+), 111 deletions(-)
New commits:
commit b57fe6473d5b1c44910a4f87d49efbaa0d27e5e4
Author: William Brown <firstyear@redhat.com>
Date: Tue Apr 26 18:07:57 2016 +1000
Ticket 48770 - Improve extended op plugin handling
Bug Description: In plugin.c we had a number of in-efficenty control paths, and
loops that were repeated.
Fix Description: This reduces code duplication, and looping in un-neccesary
places.
https://fedorahosted.org/389/ticket/48770
Author: wibrown
Review: nhosoi (Thanks!)
diff --git a/ldap/servers/slapd/extendop.c b/ldap/servers/slapd/extendop.c
index 50506a5..5154602 100644
--- a/ldap/servers/slapd/extendop.c
+++ b/ldap/servers/slapd/extendop.c
@@ -206,6 +206,7 @@ do_extended( Slapi_PBlock *pb )
{
char *extoid = NULL, *errmsg;
struct berval extval = {0};
+ struct slapdplugin *p = NULL;
int lderr, rc;
ber_len_t len;
ber_tag_t tag;
@@ -334,23 +335,19 @@ do_extended( Slapi_PBlock *pb )
slapi_pblock_set( pb, SLAPI_EXT_OP_REQ_VALUE, &extval );
slapi_pblock_set( pb, SLAPI_REQUESTOR_ISROOT, &pb->pb_op->o_isroot);
- /* wibrown 201603 I want to rewrite this to get plugin p, and use that
- * rather than all these plugin_call_, that loop over the plugin lists
- * We do "get plugin (oid).
- * then we just hand *p into the call functions.
- * much more efficient! :)
- */
-
- slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c calling plugins ... \n");
+ rc = plugin_determine_exop_plugins( extoid, &p );
+ slapi_log_error(SLAPI_LOG_TRACE, NULL, "exendop.c plugin_determine_exop_plugins rc %d\n", rc);
+ if (rc == SLAPI_PLUGIN_EXTENDEDOP && p != NULL) {
+ slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c calling plugin ... \n");
+ rc = plugin_call_exop_plugins( pb, p);
- rc = plugin_call_exop_plugins( pb, extoid, SLAPI_PLUGIN_EXTENDEDOP);
+ slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c called exop, got %d \n", rc);
- slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c called exop, got %d \n", rc);
+ } else if (rc == SLAPI_PLUGIN_BETXNEXTENDEDOP && p != NULL) {
- if (rc == SLAPI_PLUGIN_EXTENDED_NOT_HANDLED) {
- slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c calling betxn plugins ... \n");
+ slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c calling betxn plugin ... \n");
/* Look up the correct backend to use. */
- Slapi_Backend *be = plugin_extended_op_getbackend( pb, extoid );
+ Slapi_Backend *be = plugin_extended_op_getbackend( pb, p );
if ( be == NULL ) {
slapi_log_error(SLAPI_LOG_FATAL, NULL, "extendop.c plugin_extended_op_getbackend was unable to retrieve a backend!!!\n");
@@ -368,7 +365,7 @@ do_extended( Slapi_PBlock *pb )
if (txn_rc) {
slapi_log_error(SLAPI_LOG_FATAL, NULL, "exendop.c Failed to start be_txn for plugin_call_exop_plugins %d\n", txn_rc);
} else {
- rc = plugin_call_exop_plugins( pb, extoid, SLAPI_PLUGIN_BETXNEXTENDEDOP);
+ rc = plugin_call_exop_plugins( pb, p );
slapi_log_error(SLAPI_LOG_TRACE, NULL, "extendop.c called betxn exop, got %d \n", rc);
if (rc == LDAP_SUCCESS || rc == SLAPI_PLUGIN_EXTENDED_SENT_RESULT) {
/* commit */
@@ -387,7 +384,6 @@ do_extended( Slapi_PBlock *pb )
if (be_pb != NULL) {
slapi_pblock_destroy(be_pb); /* Clean up after ourselves */
}
- slapi_log_error(SLAPI_LOG_TRACE, NULL, "exendop.c plugin_call_exop_plugins rc final %d\n", rc);
} /* if be */
}
@@ -396,6 +392,7 @@ do_extended( Slapi_PBlock *pb )
lderr = LDAP_PROTOCOL_ERROR; /* no plugin handled the op */
errmsg = "unsupported extended operation";
} else {
+ slapi_log_error(SLAPI_LOG_FATAL, NULL, "extendop.c failed with result %d \n", rc);
errmsg = NULL;
lderr = rc;
}
diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c
index 96169e6..43ea1e3 100644
--- a/ldap/servers/slapd/plugin.c
+++ b/ldap/servers/slapd/plugin.c
@@ -475,6 +475,46 @@ plugin_call_entryfetch_plugins(char **entrystr, uint *size)
}
/*
+ * plugin_determine_exop_plugins
+ *
+ * A call to this function will determine the correct plugin that is required
+ * based on the extended operation ID.
+ *
+ * extoid : The extended operation oid as a *char
+ * plugin: A pointer to a struct slapdplugin *. IE &*p This will be set by the function.
+ * return: SLAPI_PLUGIN_EXTENDED_NOT_HANDLED if no plugin. Otherwise, the SLAPI_PLUGIN_* type that the plugin is.
+ */
+int
+plugin_determine_exop_plugins( const char *oid, struct slapdplugin **plugin)
+{
+ struct slapdplugin *p = NULL;
+ int list_type = 0;
+ int i = 0;
+ int l = 0;
+ int rc = SLAPI_PLUGIN_EXTENDED_NOT_HANDLED;
+
+ int list_types[] = {PLUGIN_LIST_EXTENDED_OPERATION, PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION};
+
+ for ( l = 0; l < 2; ++l ) {
+ list_type = list_types[l];
+
+ for ( p = global_plugin_list[list_type]; p != NULL; p = p->plg_next ) {
+ if ( p->plg_exhandler != NULL && p->plg_exoids != NULL ) {
+ for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
+ if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
+ *plugin = p;
+ rc = p->plg_type;
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ return rc;
+}
+
+/*
* call extended operation plugins
*
* return SLAPI_PLUGIN_EXTENDED_SENT_RESULT if one of the extended operation
@@ -485,50 +525,21 @@ plugin_call_entryfetch_plugins(char **entrystr, uint *size)
* returned by the plugins we called).
*/
int
-plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid, int whichtype )
+plugin_call_exop_plugins( Slapi_PBlock *pb, struct slapdplugin *p )
{
- struct slapdplugin *p;
- int i, rc;
- int list_type;
+ int rc = LDAP_SUCCESS;
int lderr = SLAPI_PLUGIN_EXTENDED_NOT_HANDLED;
- if (whichtype == SLAPI_PLUGIN_EXTENDEDOP) {
- list_type = PLUGIN_LIST_EXTENDED_OPERATION;
- } else if (whichtype == SLAPI_PLUGIN_BETXNEXTENDEDOP) {
- list_type = PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION;
- } else {
- slapi_log_error(SLAPI_LOG_FATAL, NULL, "plugin_call_exop_plugins unknown plugin list type %d\n", whichtype);
- return( lderr );
- }
-
- for ( p = global_plugin_list[list_type]; p != NULL; p = p->plg_next ) {
- if ( p->plg_exhandler != NULL && p->plg_type == whichtype ) {
- if ( p->plg_exoids != NULL ) {
- for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
- if ( strcasecmp( oid, p->plg_exoids[i] )
- == 0 ) {
- break;
- }
- }
- if ( p->plg_exoids[i] == NULL ) {
- continue;
- }
- }
-
- slapi_pblock_set( pb, SLAPI_PLUGIN, p );
- set_db_default_result_handlers( pb );
- if ( (rc = (*p->plg_exhandler)( pb ))
- == SLAPI_PLUGIN_EXTENDED_SENT_RESULT ) {
- return( rc ); /* result sent */
- } else if ( rc != SLAPI_PLUGIN_EXTENDED_NOT_HANDLED ) {
- /*
- * simple merge: report last real error
- */
- if ( lderr == SLAPI_PLUGIN_EXTENDED_NOT_HANDLED
- || rc != LDAP_SUCCESS ) {
- lderr = rc;
- }
- }
+ slapi_pblock_set( pb, SLAPI_PLUGIN, p );
+ set_db_default_result_handlers( pb );
+ if ( (rc = (*p->plg_exhandler)( pb )) == SLAPI_PLUGIN_EXTENDED_SENT_RESULT ) {
+ return( rc ); /* result sent */
+ } else if ( rc != SLAPI_PLUGIN_EXTENDED_NOT_HANDLED ) {
+ /*
+ * simple merge: report last real error
+ */
+ if ( lderr == SLAPI_PLUGIN_EXTENDED_NOT_HANDLED || rc != LDAP_SUCCESS ) {
+ lderr = rc;
}
}
@@ -550,75 +561,51 @@ const char *
plugin_extended_op_oid2string( const char *oid )
{
struct slapdplugin *p;
- int i, j, l, list_type;
- const char *rval = NULL;
- int list_types[] = {PLUGIN_LIST_EXTENDED_OPERATION, PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION};
+ int j = 0;
+ int l = 0;
+ int rc = 0;
+ const char *rval = NULL;
+
+ rc = plugin_determine_exop_plugins( oid, &p);
+ if ((rc == SLAPI_PLUGIN_EXTENDEDOP || rc == SLAPI_PLUGIN_BETXNEXTENDEDOP) && p != NULL ) {
+ /* We have the plugin, p set, so lets fill it in */
+ if ( NULL != p->plg_exnames ) {
+ for ( j = 0; p->plg_exnames[j] != NULL; ++j ) {
+ /* I'm not sure what this does ....*/
+ ;
+ }
+ rval = p->plg_exnames[j]; /* OID-related name */
+ }
- /* I feel there may be a better way to achieve this, but it works. */
- for ( l = 0; l < 2; ++l ) {
- list_type = list_types[l];
- for ( p = global_plugin_list[list_type]; p != NULL; p = p->plg_next ) {
- if ( p->plg_exhandler != NULL && p->plg_exoids != NULL ) {
- for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
- if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
- if ( NULL != p->plg_exnames ) {
- for ( j = 0; j < i && p->plg_exnames[j] != NULL; ++j ) {
- ;
- }
- rval = p->plg_exnames[j]; /* OID-related name */
- }
-
- if ( NULL == rval ) {
- if ( NULL != p->plg_desc.spd_id ) {
- rval = p->plg_desc.spd_id; /* short name */
- } else {
- rval = p->plg_name; /* RDN */
- }
- }
- break;
- }
- } /* for */
- } /* If */
- } /* for p in global_plugin list */
- } /* list type */
+ if ( NULL == rval ) {
+ if ( NULL != p->plg_desc.spd_id ) {
+ rval = p->plg_desc.spd_id; /* short name */
+ } else {
+ rval = p->plg_name; /* RDN */
+ }
+ }
+ }
return( rval );
}
Slapi_Backend *
-plugin_extended_op_getbackend( Slapi_PBlock *pb, char *oid )
+plugin_extended_op_getbackend( Slapi_PBlock *pb, struct slapdplugin *p )
{
- struct slapdplugin *p;
- int i;
+ // struct slapdplugin *p;
int rc;
/* This could be an error type, but for now we expect the caller to check
* that it's not null
*/
Slapi_Backend *result = NULL;
- for ( p = global_plugin_list[PLUGIN_LIST_BE_TXN_EXTENDED_OPERATION]; p != NULL; p = p->plg_next ) {
- if ( p->plg_be_exhandler != NULL && p->plg_type == SLAPI_PLUGIN_BETXNEXTENDEDOP ) {
- if ( p->plg_exoids != NULL ) {
- for ( i = 0; p->plg_exoids[i] != NULL; i++ ) {
- if ( strcasecmp( oid, p->plg_exoids[i] ) == 0 ) {
- break;
- }
- }
- if ( p->plg_exoids[i] == NULL ) {
- continue;
- }
- }
- rc = (*p->plg_be_exhandler)( pb, &result );
- if (rc != LDAP_SUCCESS) {
- /* Do we need to do anything? Or it is the parents job? */
- result = NULL;
- }
- break;
- }
+ rc = (*p->plg_be_exhandler)( pb, &result );
+ if (rc != LDAP_SUCCESS) {
+ /* Do we need to do anything? Or it is the parents job? */
+ result = NULL;
}
-
return( result );
}
diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h
index 255e4bd..f4a5eab 100644
--- a/ldap/servers/slapd/proto-slap.h
+++ b/ldap/servers/slapd/proto-slap.h
@@ -879,8 +879,9 @@ void global_plugin_init();
int plugin_call_plugins( Slapi_PBlock *, int );
int plugin_setup(Slapi_Entry *plugin_entry, struct slapi_componentid *group,
slapi_plugin_init_fnptr initfunc, int add_to_dit, char *returntext);
-int plugin_call_exop_plugins( Slapi_PBlock *pb, char *oid, int whichtype );
-Slapi_Backend * plugin_extended_op_getbackend( Slapi_PBlock *pb, char *oid);
+int plugin_determine_exop_plugins( const char *oid, struct slapdplugin **plugin );
+int plugin_call_exop_plugins( Slapi_PBlock *pb, struct slapdplugin *p );
+Slapi_Backend * plugin_extended_op_getbackend( Slapi_PBlock *pb, struct slapdplugin *p);
const char *plugin_extended_op_oid2string( const char *oid );
void plugin_closeall(int close_backends, int close_globals);
void plugin_startall(int argc, char **argv, char **plugin_list);
--
389-commits mailing list
389-commits@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/389-commits@lists.fedoraproject.org
No comments:
Post a Comment