[Kamailio-Devel] [ openser-Patches-2018652 ] Export an API from the textops module.

[SourceForge.net]

Patches item #2018652, was opened at 2008-07-15 13:19
Message generated for change (Comment added) made by henningw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743022&aid=2018652&group_id=139143

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: modules
Group: ver devel
>Status: Closed
>Resolution: Accepted
Priority: 5
Private: No
Submitted By: Ardjan Zwartjes (ardjan_zwartjes)
Assigned to: Henning Westerholt (henningw)
Summary: Export an API from the textops module.

Initial Comment:
I'm currently developing some openser modules and frequently encounter situations where I need functionality similar to things implemented in the textops module. Instead of copying functionality from textops it would be nice to have an API exported by the textops module to include in other modules (similar to what is done in the TM module and RR module). 

After talking about this option on the devel mailing list (http://lists.openser.org/pipermail/devel/2008-July/014712.html) I have created the attached patch against openser 1.3.2 TLS (the version I'm using). I have tested this code and it seems to work.

Currently I only export the following functions:
append_hf
remove_hf
search_append
search

which covers most of the functionality I need, if you guys think this functionality is useful and can be added to OpenSER I'm willing to spend some more time to add the other textops functions ass well and update the documentation to include the exported functions.

I hope this code can be used.

Kind regards,
Ardjan Zwartjes

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

>Comment By: Henning Westerholt (henningw)
Date: 2008-08-11 06:46

Message:
Logged In: YES 
user_id=337916
Originator: NO

Hi Ardjan,

i've commited your patch to the trunk, thanks again. I also added the
pkg_free calls you suggested. Please review, test and let me know if they
are issues.

Henning

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

Comment By: Ardjan Zwartjes (ardjan_zwartjes)
Date: 2008-07-22 08:59

Message:
Logged In: YES 
user_id=1744253
Originator: YES

While using the patch I proposed here, I encountered some memory leaks. In
the file api.c in the functions search_append_api and search_api param
needs to be freed after free_fixup_str2regexp. So in both functions the
line

pkg_free(param);

needs to be added after the call to free_fixup_str2regexp.

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

Comment By: Ardjan Zwartjes (ardjan_zwartjes)
Date: 2008-07-16 14:34

Message:
Logged In: YES 
user_id=1744253
Originator: YES

I have created a new patch for SVN trunk (4485 I believe) with the
suggested changes. It compiles without warnings and I think it just might
work but I guess it does needs some tests.
File Added: textops-trunk.diff

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

Comment By: Henning Westerholt (henningw)
Date: 2008-07-16 09:06

Message:
Logged In: YES 
user_id=337916
Originator: NO

Hi Ardjan,

thanks for the patch. I've did a short review, and found a few (small)
issues:

- you make mod_init non-static, this is not necessary i think
- you use DEBUG: in some of the log statements, e.g. LM_DBG("DEBUG:.."
This is 
  not necessary, this prefix is added automatically
- it would be nice if you could check for null pointer in load_textops
- you noticed that this patch is based against the 1.3 branch, it would be

  better to have this against trunk
- the comment in api.h is a little bit too "long", perhaps you can compact

  some lines


For me its ok if the API contains only this four functions, if some others
are needed, they could be added as well. If nobody objects against this,
then this could be merged after 1.4.0 is released.

You should then revert the changes to the functions that are not exported
at the moment (leave them static, don't expose them in the header file) and
also add some documentation about the exported functions. But first lets
see if some there are other opinions about this..


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

Comment By: Henning Westerholt (henningw)
Date: 2008-07-16 08:50

Message:
Logged In: YES 
user_id=337916
Originator: NO

moved to patches section

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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=743022&aid=2018652&group_id=139143