[Home]

Summary:ASTERISK-09660: Janitor Project - ast_debug() conversion
Reporter:Caio Begotti (caio1982)Labels:
Date Opened:2007-06-12 12:34:27Date Closed:2007-06-14 14:39:22
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-res.diff
( 1) astlog_app_changes_fixed.diff
( 2) astlog_cdr_changes.diff
( 3) astlog_chan1_changes.patch
( 4) astlog_chan2_changes.patch
( 5) astlog_chansip_changes.patch
( 6) astlog_codec_changes.diff
( 7) astlog_format_changes.diff
( 8) astlog_funcs_changes.diff
( 9) chan_skinny_ast_debug2.patch
(10) log-main-1.patch
(11) voicemail_space_fixes.diff
Description:Reproducing the original message from Russel:

Dmitry Andrianov has created a new macro, ast_debug() which simplifies debug logging. He has also converted main/pbx.c to use it. Now, the rest of the code base needs to get converted. If you are interested in helping, volunteer to take a section of the code in the thread link below so you don't duplicate efforts.

Currently, you will see code like:

if (option_debug)
  ast_log(LOG_DEBUG, ...);

if (option debug > 3)
  ast_log(LOG_DEBUG, ...);

Now, that would look like

ast_debug(1, ...);

ast_debug(3, ...);

****** ADDITIONAL INFORMATION ******

Reference: http://www.mail-archive.com/asterisk-dev@lists.digium.com/msg27376.html
Comments:By: Michiel van Baak (mvanbaak) 2007-06-12 12:59:14

attached chan_skinny patch.
Disclaimed and all :)
By: Caio Begotti (caio1982) 2007-06-12 13:16:02

The astlog_app1_changes.diff (properly disclaimed) contains syntax changes on "ast_log(LOG_DEBUG" for the following applications:

app_chanspy.c
app_festival.c
app_alarmreceiver.c
app_db.c
app_channelredirect.c
app_directed_pickup.c
app_dial.c
app_disa.c
app_amd.c
app_followme.c
By: Michiel van Baak (mvanbaak) 2007-06-12 14:43:40

uploaded new skinny patch without the regexten/regcontext config file changes.
By: Caio Begotti (caio1982) 2007-06-12 14:50:05

Another patch (astlog_app2_changes2.diff) that changes ast_log in a bunch of applications. Could someone delete astlog_app2_changes.diff, please? I got a typo on it.

app_zapras.c
app_stack.c
app_talkdetect.c
app_mp3.c
app_meetme.c
app_ices.c
app_minivm.c
app_macro.c
app_nbscat.c
app_waitforsilence.c
app_zapbarge.c
app_osplookup.c
app_zapscan.c
app_record.c


By: Michiel van Baak (mvanbaak) 2007-06-12 15:37:22

I'm working on the files in channels/ ....
By: Michiel van Baak (mvanbaak) 2007-06-12 16:15:46

astlog_chan1_changes.path (disclaimed) contains ast_debug changes for:

chan_jingle
chan_phone
chan_features
chan_h323
chan_agent
chan_iax2
chan_misdn
chan_alsa
chan_nbs
chan_mgcp
chan_local
chan_gtalk
By: critch (critch) 2007-06-12 16:30:34

Here is the changes for the res/ tree with the exception of res_eventtest.c.

I'm not sure where option_debug is defined and the buld complains about it after the change. As it is, it always runs the ast_log and isn't conditional.
By: Michiel van Baak (mvanbaak) 2007-06-12 17:12:07

Here's a patch dedicated for chan_sip because it's huge.
By: Michiel van Baak (mvanbaak) 2007-06-12 17:44:15

astlog_chan2_changes.patch added for chan_zap and chan_vpb
All of channels/ tree is done now.
By: Dmitry Andrianov (dimas) 2007-06-12 19:22:58

First part of main/

M      main/config.c
M      main/cdr.c
M      main/channel.c
M      main/app.c
M      main/dns.c
M      main/db.c
M      main/acl.c
M      main/asterisk.c
M      main/dsp.c
M      main/frame.c
M      main/devicestate.c
M      main/enum.c
M      main/dial.c
M      main/adsistub.c
M      main/file.c
M      main/callerid.c

Notes:
1. there were unconditional calls to ast_log(LOG_DEBUG) - in the callerid.c and dial.c just to name few. These now go with level=1 which means no output unless debug actually enabled.
2. main/channel.c functions ast_channel_trylock, ast_channel_lock and ast_channel_unlock a bit disagree on log level used when reporting similar conditions. I left these as in the original code but I think some common level shoul be used.
3. In the functions above I sometimes used

                      ast_debug(3, ":::=== Still have %d locks (recursive)\n", count);

(note level 3) although original code was:

                      if (option_debug)
                              ast_log(LOG_DEBUG, ":::=== Still have %d locks (recursive)\n", count);

(that is level 1). This is because there is higher level "if" checking that option_debug > 2 so even if inner check is just "(option_debug)", the actual level is 3.
By: Caio Begotti (caio1982) 2007-06-12 19:48:07

Just uploaded the patch for changes in CDR modules (with the counting sequence done right this time). I'm fixing the applications' patch and will re-upload it ASAP.
By: Caio Begotti (caio1982) 2007-06-12 20:49:15

I'd like to request the removel of all astlog_app* attachments. They all have incorrect counting sequences for the levels. ONLY astlog_app_changes_fixed.diff is ok. This patch contains the last applications to be converted as well.


By: Caio Begotti (caio1982) 2007-06-12 21:01:52

Ok, I just noticed that app_voicemail.c has (15) missing weird ast_log with blank space before parenthesis then LOG_DEBUG. The patch voicemail_space_fixes.diff solves that after aplying my patch for all applications.
By: Caio Begotti (caio1982) 2007-06-12 21:07:46

Uploaded format, functions and codecs patches. Small ones.


By: Russell Bryant (russell) 2007-06-14 14:39:21

All of these patches have been committed to trunk in revision 69327.  Thanks!