[asterisk-dev] -netsec sip_destroy_hook annoyance, libmidcom proposed change

[Peter Beckman]

On Sat, 2 Dec 2006, Peter Beckman wrote:

> On Sat, 2 Dec 2006, Johansson Olle E wrote:
>
>> 
>> 2 dec 2006 kl. 21.52 skrev Peter Beckman:
>> 
>>> I'm going to be using a Ranch Networks box in front of asterisk, which
>>> requires -netsec and libmidcom.  I'm building and testing the asterisk
>>> boxes without the Ranch boxes to make sure they work first.
>>> 
>>> I disabled midcom in the midcom.conf by putting "midcomnat = no" while it
>>> was not needed; it was preventing calls from going through since no
>>> firewall existed between it and the net.
>>> 
>>> Now I get these messages every few seconds:
>>>
>>>    Dec  2 15:23:38 ERROR[12890]: res_netsec.c:1058 
>>> midcom__sip_destroy_hook: Midcom: Module not enabled
>>>    Midcom: Module currently disabled
>>> 
>> This is surely no "ERROR" and needs to be moved away to another class.
>
> LOG_DEBUG perhaps?
>
> Is there a formal definitions of the verbosity levels and what is
> appropriate at each level?
>
> Original:
>
>    if (!midcom_enabled) {
>      ast_log(LOG_ERROR, "Midcom: Module not enabled\n");
>      if (option_verbose > 1)
>        ast_verbose("Midcom: Module currently disabled\n");
>      return 1;
>    }
>
> Proposed change:
>
>    if (!midcom_enabled) {
>      ast_log(LOG_DEBUG, "Midcom: Module not enabled\n");
>      if (option_verbose > 300)
>        ast_verbose("Midcom: Module currently disabled\n");
>      return 1;
>    }

  Didn't notice that this is done in 5 different places.  Proposed changes
  apply to the following lines:

     602, 681, 799, 888, 1057

  Might it make sense to write a function called "is_midcom_enabled" and
  have it return the value of "midcom_enabled" and do the logging there?
  Unifies 5 pieces of code that are exactly the same.  Plus could be used in
  these additional places where midcom_enabled is checked:

     1126, 1134, 1142, 1149, 1156, 1164

Beckman
---------------------------------------------------------------------------
Peter Beckman                                                  Internet Guy
beckman at purplecow.com                             http://www.purplecow.com/
---------------------------------------------------------------------------