[OpenSER-Devel] Request for comment: module interface extension

[Henning Westerholt]

On Monday 17 December 2007, Dan Pascu wrote:
> > is also not optimal. Here happens also some sort of information hiding,
> > the actually used variables vs. the boilerplate code.. And the current
> > interface with char* that gets casted around to fit to arbitrary
> > variables is also not really clean..
>
> While not optimal, is far less disruptive that what you propose. At least
> I can give my function parameters meaningful names and have an idea of
> what the function receives and what expects. With the array you propose I
> have no clue unless I inspect the inner code.

Hi Dan,

sure, this is a major change of the interface. But if we want a more flexible 
module functions, then this is invevitable, regardless if we use char** or 
va_list. The alternative would be to use c++ function overloading, but this 
would be probably not accepted. ;-)

> > In regards to the second point you've raised, the intermodule function
> > calls:
> >
> > Modules that holds functions that needs to be used from outside should
> > export a clean API struct with a 'bind' function, like the tm or auth
> > modules, in my opinion. No modules should call functions that are only
> > exported via the config script. This is not type safe at all. And the
> > module functions exported with this type of API don't need to adhere to
> > the interface of the config script.
>
> Sometimes that's not an option as the same function is made available to
> the script and used from other modules as well. In your model this means
> we have to define a function and a wrapper because we have to export the
> same function under 2 different calling conventions.

This is the scheme actually used from tm, auth, rr and other module, this 
would not change because of the proposed change.

> Regarding this change, C already has a mechanism to indicate that a
> function can receive a variable number of arguments, by using the
> ellipsis notation and varags. Why not use that which will allow to have
> functions with as many arguments you want without the awkwardness of
> passing arrays for arguments.

I take a look into this mechanism. You simply change the char** to the va_list 
type. Additionally you need to call va_start and va_end at every access to 
the commands. You need to carry the number of parameters with you in both 
cases, as va_args return undefined results if you access parameters that are 
not included in the actual function call. You can not access the parameters 
simply here with array indexes like command[0], its necessary to walk linear 
over the list every time. You need to specify the actual type of every 
parameter that you want to access for the va_arg call. 
I do not think this make the whole stuff more transparent or convinient.

> All the function modules have to do is to 
> declare themselves as accepting a variable number of arguments even
> though they do not actually need to parse and use those extra arguments.
> And the script parser will pass as many arguments to the function as it
> has received from the script.
> This even has the advantage that we no longer have to define multiple
> prototypes in the module's cmd_export_t structure if we have a function
> with optional arguments. The function is declared only once with a
> prototype including the minimum required arguments and the rest will be
> determined at runtime if present using varargs.

The same advantage applies for the char** case. The vaargs are not completly 
evaluated at runtime, there actually defined with some macros that reserve 
the space for the additional variables. Thats the reasons that you get 
undefined results if you access more variables then given at on function 
call.

I think its mainly a matter of taste. Both ways of specifying additional 
parameters are roughly equivalent. I would go with the char** case, because 
its more comfortable, as you don't need the va_start, va_end stuff, and you 
can access the commands just with an array index.

Cheers,

Henning