[asterisk-dev] stringfield API advice sought

[Luigi Rizzo]

I have completed the cleanup of stringfields and plan to merge it
(to trunk only, except for a couple of bugfixes) within the next day or two.    
A summary of changes is below:

+ largely simplified the stringfields API and its documentation.
  Basically i implemented the features mentioned in my original
  email and attached at the end of this file;
+ identified and fixed a couple of memory leaks in app_meetme and res_features
  (these will be merged to 1.4 too; 1.2 doesn't have stringfields);
+ marginally reduced the memory footprint by saving one size_t in the 
  stringfield descriptor;
+ return memory to the system when clearing all fields (currently
  only chan_sip uses this feature);
+ removed code that triggers a bug with gcc 2.95 :)

Apart from the bugfix above, I don't think there is any need to merge
the other changes into 1.4, but I can do it if there is demand.

The change touches a relatively small number of files: the next two

        include/asterisk/stringfields.h and main/utils.c 

implement stringfields, while clients are

        channels/chan_sip.c, channels/chan_iax2.c, main/pbx.c
        main/channel.c, res/res_features.c apps/app_meetme.c

If someone wants to look at the details, most of them are in
the team/rizzo/video_v2 branch.
have a look at include/asterisk/stringfields.h and main/utils.c for
the new code, and for most of the diffs you can try the following:

svn diff http://svn.digium.com/svn/asterisk/team/rizzo/video_v2 -r 87532:87810

	cheers
	luigi


On Tue, Oct 30, 2007 at 12:01:23PM -0700, Luigi Rizzo wrote:
> During today's work to cleanup stringfields, i am looking at the
> API to see if there are entries that we can trim (a detailed usage
> statistic is at the end of this email).
> 
> I have the following proposal:
> 
> * Of course the basic external APIs
>   are not being discussed - they are necessary:
> 
>     ast_string_field_init()
>     ast_string_field_set()
>     ast_string_field_build()
>     ast_string_field_free_memory()	to free storage
> 	(please look at the previous msg on stringfields; this
> 	should replace ast_string_field_free_all() and
> 	ast_string_field_free_pools()
> 
> * I have some concerns on the usefulness of ast_string_field_free() because
>   you don't really recover memory, just replace it with something else.
>   I would rather replace the few current usages with something like
> 
>     ast_string_field_set(x, field, NULL)
> 
>   which shouldn't be too different (btw, we need to make the function
>   safe when given a NULL pointer);
> 
> * I am a bit uncomfortable with the field_index_ ... function, because i
>   think the API is quite innatural, especially when exported to external
>   code. If stringfields are much like ordinary pointers (except that you
>   need a bit of marshalling when writing to them) then i would rather
>   have something like
> 	ast_string_field_ptr_set(x, &x->my_field, value)
> 
>   which takes the address of a field to play with. This would cover
>   the (single) instance of it in the asterisk code, in chan_sip.c,
>   and would also make ast_string_field_index() unnecessary because
>   you just use the & contruct for it.
>   Internal functions could also be changed (and simplified) accordingly.
>