7:40 a.m.
When is it safe to leave sc_writewait uninitialized? The field
seems to have emerged in the middle of RE24, and I don't see any
flags which say it should/should not be used.
Since not setting it crashed back-relay in ITS#8428, does that
mean everything which creates a slap_callback should set it?
Seems unfortunate for third-party modules...
Also, I think this should be documented.
--
Hallvard
9:03 a.m.
Hallvard Breien Furuseth wrote:
When is it safe to leave sc_writewait uninitialized? The field
seems to have emerged in the middle of RE24, and I don't see any
flags which say it should/should not be used.
Where would such flags reside?
Since not setting it crashed back-relay in ITS#8428, does that
mean everything which creates a slap_callback should set it?
Yes.
Seems unfortunate for third-party modules...
Also, I think this should be documented.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
9:56 a.m.
Howard Chu wrote:
Hallvard Breien Furuseth wrote:
> When is it safe to leave sc_writewait uninitialized? The field
> seems to have emerged in the middle of RE24, and I don't see any
> flags which say it should/should not be used.
Where would such flags reside?
slap_callback.sc_flags. If it existed it could get a "the sc_writewait
field is valid" bit. Then the struct could have been safely extended
without breaking other modules - provided they were recompiled, anyway.
> Since not setting it crashed back-relay in ITS#8428, does that
> mean everything which creates a slap_callback should set it?
Yes.
Then most callback code in slapd must be fixed. Now I don't understand
why slapd still works as well as it does...
Instead of sc_flags, maybe we should add a word 'sc_signature' for now:
back-mdb must set it to a particular value which indicates that
sc_writewait is valid, and it must be cleared before freeing the
callback. That way modules which don't know about sc_writewait
_usually_ won't break, though the signature can be right once in a
while.
--
Hallvard
10:17 a.m.
Hallvard Breien Furuseth wrote:
Howard Chu wrote:
> Hallvard Breien Furuseth wrote:
>> When is it safe to leave sc_writewait uninitialized? The field
>> seems to have emerged in the middle of RE24, and I don't see any
>> flags which say it should/should not be used.
>
> Where would such flags reside?
slap_callback.sc_flags. If it existed it could get a "the sc_writewait
field is valid" bit. Then the struct could have been safely extended
without breaking other modules - provided they were recompiled, anyway.
>> Since not setting it crashed back-relay in ITS#8428, does that
>> mean everything which creates a slap_callback should set it?
>
> Yes.
Then most callback code in slapd must be fixed. Now I don't understand
why slapd still works as well as it does...
Because most callback users do a default initialization, e.g.
slap_callback cb = { xx };
back-relay was odd in this respect.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
11:07 a.m.
Howard Chu wrote:
Hallvard Breien Furuseth wrote:
> Then most callback code in slapd must be fixed. Now I don't understand
> why slapd still works as well as it does...
Because most callback users do a default initialization, e.g.
slap_callback cb = { xx };
Oh, right... New problem, though: If a a third-party module sets
sc_private in such an initializer, that now sets sc_writewait instead.
I don't see that in OpenLDAP code - it is C90-compatible and thus
requires constant expressions in struct initializers.
Anyway, branch "writewait" in my UiO repo patches more missed cases.
--
Hallvard
11:18 a.m.
Hallvard Breien Furuseth wrote:
I wrote:
> Oh, right... New problem, though: If a a third-party module sets
> sc_private in such an initializer, that now sets sc_writewait instead.
I mean, and old module which predates sc_writewait.
Hm yeah, I suppose we could just move sc_writewait to the end of the struct.
Probably should have done that in the first place.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
11:18 p.m.
I lost this thread (from May 19). Summarized in ITS#8435
plus a new suggestion: Revert sc_writewait and get it
done with a field somewhere else, maintained by callbacks.
Dunno if that's feasible.
11:26 p.m.
There a lot of bugs around slap_callback.sc_writewait
https://github.com/ReOpen/ReOpenLDAP/commit/80c9667d823475710bb6c94a7735c...
Regards,
Leonid.
2016-06-06 9:18 GMT+03:00 Hallvard Breien Furuseth <h.b.furuseth(a)usit.uio.no>:
I lost this thread (from May 19). Summarized in ITS#8435
plus a new suggestion: Revert sc_writewait and get it
done with a field somewhere else, maintained by callbacks.
Dunno if that's feasible.
11:45 p.m.
On 06/06/16 08:26, Леонид Юрьев wrote:
There a lot of bugs around slap_callback.sc_writewait
https://github.com/ReOpen/ReOpenLDAP/commit/80c9667d823475710bb6c94a7735c...
Last I heard, we should stay away from your repo duo to an IPR conflict.
Can you summarize, if it's more than what I got?
slapd: backover.c, syncrepl.c.
slapd/slapi: slapi_overlay.c.
slapd/overlays: constraint.c, dds.c, dynlist.c, memberof.c, pcache.c.
contrib/slapd-modules: authzid/authzid.c.
That's from <git://git.uio.no/u/hbf/openldap.git> branch "writewait".
2:55 p.m.
Hallvard Breien Furuseth wrote:
I lost this thread (from May 19). Summarized in ITS#8435
plus a new suggestion: Revert sc_writewait and get it
done with a field somewhere else, maintained by callbacks.
Dunno if that's feasible.
There is no "somewhere else" to use.
I see no reason to revert the change. We can simply add sc_writewait to the
end of the structure, which is the normal practice for adding new fields to
existing structs.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
5:57 a.m.
On 08. juni 2016 23:55, Howard Chu wrote:
Hallvard Breien Furuseth wrote:
> I lost this thread (from May 19). Summarized in ITS#8435
> plus a new suggestion: Revert sc_writewait and get it
> done with a field somewhere else, maintained by callbacks.
> Dunno if that's feasible.
There is no "somewhere else" to use.
I see no reason to revert the change.
The reason would be trying to not break third-party modules, like I've said.
But the problem is 1.5 years old and hopefully such a problem would have
been reported by now, so maybe there are very few affected modules out there.
We can simply add sc_writewait to the
end of the structure, which is the normal practice for adding new fields to
existing structs.
--
Hallvard
9:54 a.m.
Hallvard Breien Furuseth wrote:
On 08. juni 2016 23:55, Howard Chu wrote:
> Hallvard Breien Furuseth wrote:
>> I lost this thread (from May 19). Summarized in ITS#8435
>> plus a new suggestion: Revert sc_writewait and get it
>> done with a field somewhere else, maintained by callbacks.
>> Dunno if that's feasible.
>
> There is no "somewhere else" to use.
>
> I see no reason to revert the change.
The reason would be trying to not break third-party modules, like I've said.
But the problem is 1.5 years old and hopefully such a problem would have
been reported by now, so maybe there are very few affected modules out there.
Exactly.
> We can simply add sc_writewait to the
> end of the structure, which is the normal practice for adding new fields to
> existing structs.
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
2175
days inactive
2196
days old
12 comments
3 participants
participants (3)
-
Hallvard Breien Furuseth -
Howard Chu -
Леонид Юрьев