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.