Re: (ITS#6739) broken do_syncrep2()

[hyc@symas.com]

h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS:
> URL:
> Submission from: (NULL) (129.240.6.233)
> Submitted by: hallvard
>
>
> slapd/syncrepl.c:do_syncrep2() initializes many variables once which
> should have been initialized inside the loop.
>
> I'll add the initializations I can figure out, and move most variables
> into the loop for readability.
>
> TODO:
>
> Variables 'm' and 'refreshDeletes' may still be wrong.  In particular
> 'm'.  It "feels" like it should be local to the loop, but there's no
> logic that I can see to prevent previous value from being used.
> Perhaps they depend on the peer not breaking the sync protocol?
> I haven't read that myself, I'm just peering at the code.

The refreshPresent/refreshDeletes flags handling is pretty bogus, I agree. 
It's always been like that and I've never felt like taking the time to clean 
it up. It Would Be Nice if someone went thru and cleaned up the entire 
consumer. While the provider was completely rewritten for 2.3, the consumer is 
still just a series of patches on the original 2.2 code, and it was never good 
code to begin with.

> I've added three somewhat arbitrary bits of code for error handling:
> - A Debug() statement on line 847 which can likely be improved.
> - Set err and rc to LDAP_OTHER on ldap_parse_result/ldap_get_option
>    failure.  The correct fix would be proper error checks (ITS#6738),
>    but I don't know what to do on error.
>
Sounds like LDAP_OTHER is fine; ldap_get_option should never fail and if it 
does, we have no idea what's going on anyway.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/