On 11/20/2016 08:17 PM, William Brown wrote:
Hi,

I have to add some new getters to pblock.c, but I think we should talk
about how to add these.

Right now, we have a nearly ~2000 line case switch statement in
slapi_pblock_get(pb, TYPE, *void). No matter how we cut it, this is
pretty insane.
:-)  Yeah its a bit much at the moment.
 We have huge amounts of defines in slapi-plugin.h for
this, and we can't expose those to other langs (python, rust), because
they are in a C header file.

As well, case switch statements that large, at some point, it's going to
be inefficient to access. 

I think that we should try to break this up, it's just a bit much at
this point.

So for new additions I would like to use:

slapi_pblock_get_<item>(pblock, out) {
    *out = pb->item
}

This works for me, but if I may, I propose that we tweak the naming style to:

slapi_pb_get_<item>(Slapi_PBlock *pb)
slapi_pb_set_<item>(Slapi_PBlock *pb, ...)

This way we differentiate the new functions, and two, it helps keep the function name from getting too long.

Mark

This is better, as the function is exported in the .so, it's accessible
to other languages, it's simpler to implement, we stop needing more
macro definitions. 

It also has performance benefits, as these functions can be inlined by
GCC, the function lookup is likely quicker than the case switch, it's
also easier to set breakpoints on for debugging.

So for now, I want to make new additions to pblock.c in this form.

One day in the future we can tackle breaking pblock.c down from the
current case switch style, but it is not this day.

What do we think? Are we happy for me to use this style for new
additions? 



_______________________________________________
389-devel mailing list -- 389-devel@lists.fedoraproject.org
To unsubscribe send an email to 389-devel-leave@lists.fedoraproject.org