In plugin.c we make a number of calls from extendop.c for accessing exop plugins.
do_extended(...) { rc = plugin_call_exop_plugins( pb, extoid, SLAPI_PLUGIN_EXTENDEDOP); ... Slapi_Backend *be = plugin_extended_op_getbackend( pb, extoid ); ... rc = plugin_call_exop_plugins( pb, extoid, SLAPI_PLUGIN_BETXNEXTENDEDOP); }
When we call this, each of these functions, in plugin.c the following pattern is used:
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; } } .... // Use the plugin
Iterating over this loop now, with small plugin lists (4) isn't such a cost: But over time it adds up, as we are now looping over this in each of those plugin_* calls.
I would like to change this to:
struct slapdplugin *p; ... result = plugin_exop_plugin_by_oid( oid, &p ); // Sets P based on the loop above ... result = plugin_call_exop_plugin( Slapi_PBlock *pb, char *oid, struct slapdplugin *p); // this now calls p->func directly rather than iterating over the list ... Slapi_Backend *be = plugin_extended_op_getbackend( pb, extoid, p ); ... rc = plugin_call_exop_plugins( pb, extoid, p);
This will make the code flow nicer, improve and remove redundant loops and checks, and also allow some smoother decisions in extendop.c.
This is a code change only, no regressions or other changes would be needed.
attachment 0001-Ticket-48770-Improve-extended-op-plugin-handling.patch
Looks good. One minor request. I know it does not occur in the current code, but just in case, since you are checking the value of p at the line 340 and 346: {{{ 340 if (rc == SLAPI_PLUGIN_EXTENDEDOP && p != NULL) { 346 } else if (rc == SLAPI_PLUGIN_BETXNEXTENDEDOP && p != NULL) { }}} Could you initialize p (line 209) and plugin in plugin_determine_exop_plugins with NULL? {{{ 209 struct slapdplugin p; 488 plugin_determine_exop_plugins( const char oid, struct slapdplugin *plugin) }}} Otherwise, you have my ack.
Set slapd_plugin to NULL 0001-Ticket-48770-Improve-extended-op-plugin-handling.2.patch
commit b57fe64 Writing objects: 100% (8/8), 1.91 KiB | 0 bytes/s, done. Total 8 (delta 6), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git 0d1c21b..b57fe64 master -> master
Metadata Update from @firstyear: - Issue assigned to firstyear - Issue set to the milestone: 1.3.5 backlog
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/1830
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.