Rein Tollevik wrote:

On Wed, 30 Apr 2008, Howard Chu wrote:

...

rein@OpenLDAP.org wrote:

...

...

syncprov_findbase() must search the backend saved with the syncrepl operation, not the one from the operation passed as argument. The backend in the op argument can be a subordinate database, in which case the search for the base in the superior database will fail, and syncrepl consumers will be force to do a an unneccessary full refresh of the database.

OK.

...

The patch at the end should fix this. Note that both fop.o_bd and fop.o_bd->bd_info can be changed by the overlay_op_walk() call, which is the reason for the long pointer traversal to find the correct bd_info to save and restore.

...

But the overlay_op_walk call is only appropriate when the DB to be searched is the current database, and the current DB is an overlay DB structure.

Ah, the changing of the BackendDB->bd_info that takes place when overlays are called feels like an open pit I manage to fall into every time I get close to it... I wish it could be replaced in a future version.

Agreed, it would have been safer as an Op-specific field, but that would have caused quite a lot of disruption to all existing backend code.

A new patch that I hope should fix this is at the end. It always use be_search, after putting back the original bd_info if needed. I feel that using the generic be_search is better than interfering directly with the overlay code as overlay_op_walk does. I also tested for SLAP_ISOVERLAY rather than PS_IS_REFRESHING, as that appeared more generic to me. But again, I may be totally wrong here. Does this patch look better?

SLAP_IS_OVERLAY will never be true here. That flag is only set when the BackendDB being tested is a local copy of a real BackendDB structure. The structure referenced in s_op is always a real BackendDB.

In fact, if you're always going to use s_op and be_search, there's no further work needed, because the regular overlay infrastructure will always make a new local BackendDB copy itself. (And of course, some of that would be wasted effort, which is why the original code uses overlay_op_walk. Since op->o_bd is already an overlay DB, there's no need to make yet another copy for the first-search case.)