[Date Prev][Date Next]
Re: New overlay, your opinion?
Johan Jakus writes:
> I've been developing an overlay and I got to a well working solution.
> But,I would really appreciate your opinions about it before sending it to
> the contribs.
> You can see my source on my site:
Some notes after a quick scan:
- Try to stay below 80-char line length. OpenLDAP mostly uses
tab-width 4 and indentation 4, though it's gotten intermixed quite
a bit of code written with tab-width 8 code over the years.
- Also OpenLDAP code triest to stay C90-compatible, so no '//' comments
or declarations after executable statements.
- LDAP_DEBUG_ANY is for log messages which should always be output, so
parsearch would be very chatty in the log. Use LDAP_DEBUG_TRACE
for most of these messages. Sometimes LDAP_DEBUG_ARGS/ACL.
- parsearch_response() should inspect rs->sr_type see what kind of
kind of response this is - search entry, intermediate response etc.
See e.g. valsort_response. It can look at op->o_tag to see what
kind of request is being processed.
Note that 'rs->sr_un.sru_search.r_entry != NULL' will be true for an
intermediate response, because sr_un is a union and
sru_extended.r_rspoid has been set.
Unfortunately the SlapReply flags are not always reliable. We've
cleaned up a lot lately.
There are a bunch of macros like sr_entry = sr_un.sru_search.r_entry
in slapd.conf, so these expresions can be written more briefly.
- This is wrong:
Entry* dupEntry = entry_dup(rs->sr_un.sru_search.r_entry);
rs->sr_un.sru_search.r_entry = dupEntry;
rs->sr_flags = REP_ENTRY_MUSTFLUSH;
You are not obeying the flags and flushing the old entry.
You are resetting non-entry-related flags. And MUSTFLUSH
= MUSTBEFREED | MUSTRELEASE, you want MUSTFREE since you have no
slap_overinst *on = (slap_overinst *) op->o_bd->bd_info;
rs_entry2modifiable( op, rs, on );
e = rs->sr_entry; /* this is now modifiable */