[asterisk-dev] stringfield API advice sought

[Luigi Rizzo]

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.

So, any objection with making the above changes (which in my opinion
should result in greatly simplified code) ?
There is only one file affected, chan_sip.c, and only in 5 lines.

	cheers
	luigi
  
---- usage statistics for the various stringfield functions ----

INTERNAL API:			
    __ast_string_field_init			1
    __ast_string_field_alloc_space		1
    __ast_string_field_index_build_va		2
    ast_string_field_index_build		some
    ast_string_field_index_build_va		some
    ast_string_field_index_free			1

EXTERNAL
.   ast_string_field_init			many
.   ast_string_field_set			many
.   ast_string_field_build			some
.   ast_string_field_free			some
.   ast_string_field_free_pools			some
.   ast_string_field_free_all			some

    ast_string_field_build_va			1 in channel.c

    ast_string_field_index_set			1 in chan_sip.c
    ast_string_field_index			4 in chan_sip.c

------------------------------------------------------------------------