#48770 Improve extended op plugin handling
Closed: Fixed None Opened 3 years ago by firstyear.

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.


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.

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

2 years ago

Login to comment on this ticket.

Metadata