jzeleny@redhat.com writes:
Could you test if this works instead? http://folk.uio.no/hbf/ol-struct-hack-1.patch If that doesn't work, similar code elsewhere may be in danger.
This hack seems to work just fine.
Thank you.
But I'd like to ask if it's the right way to do it - I mean, as you said the code isn't run that often, so there is no need to consider performance and I think my patch offers cleaner solution.
That discussion gets a bit bigger than the problem, but anyway:
I'd say the right way to write it is the way we'd be happy to se people use it as demo/template code. So yes, even though the back-ldif code isn't executed much, it's good to use simple tricks like this anyway. As long as they are not buggy or unnecessarily complicate the code.
The silly way to do it was to use a well-known but officially dubious trick, the struct hack, with a non-standard notation: "char fname;" instead of "char fname[1];", and macroized at that.
I see several mentions on gcc.gnu.org about breaking and un-breaking the struct hack. I haven't looked closely, but for now I'll guess gcc doesn't see the original back-ldif code as the struct hack and doesn't un-break it:
The code stores a struct bvlist object in the first part of the malloced memory, thus informing the compiler that any padding bytes in that bvlist object have unspecified values. Thus I guess the compiler is free to ignore whatever back-ldif puts in these bytes.
Reading gcc.gnu.org about the struct hack was a bit unsettling though, so it seemed better to avoid it. Maybe others in the project have different opinion, I don't know. I seem to be arguing with myself about readability anyway:-)
As for malloc, slapd's main problem with malloc is memory fragmentation. Performance comes second.
And for both memory fragmentation and speed, the "righter" way for small data freed by the same thread would not be malloc() but ber_memalloc_x() or slap_sl_malloc() with a non-NULL context parameter. Oh well, maybe later - if anyone cares.