[Home]

Summary:ASTERISK-10302: [patch] use NEW_CLI for CLI commands.
Reporter:Eliel Sardanons (eliel)Labels:
Date Opened:2007-09-14 16:02:06Date Closed:2007-10-15 08:25:01
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Resources/res_features
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_meetme.c.patch
( 1) app_minivm.c.patch
( 2) app_mixmonitor.c.patch
( 3) app_osplookup.c.patch
( 4) app_osplookup-rev83558.patch
( 5) app_playback.c.patch
( 6) app_queue.c.patch
( 7) app_rpt.c.patch
( 8) app_voicemail.c.patch
( 9) asterisk.c.patch
(10) astmm.c.patch
(11) astobj2.c.patch
(12) cdr.c.patch
(13) chan_agent.c.patch
(14) chan_alsa.c.patch
(15) chan_features.c.patch
(16) chan_gtalk.c.patch
(17) chan_iax2.c.patch
(18) chan_local.c.patch
(19) chan_mgcp.c.patch
(20) chan_mgcp.c.patch2
(21) chan_oss.c.patch
(22) chan_sip.c.patch
(23) chan_skinny.c.patch
(24) chan_zap.c.patch
(25) channel.c.patch
(26) cli.c.patch
(27) db.c.patch
(28) dnsmgr.c.patch
(29) file.c.patch
(30) frame.c.patch
(31) http.c.patch
(32) iax2-provision.c.patch
(33) image.c.patch
(34) logger.c.patch
(35) manager.c.patch
(36) NEW_CLI-missing.txt
(37) pbd_dundi.c.patch
(38) pbx_ael.c.patch
(39) pbx_config.c.patch
(40) pbx.c.patch
(41) res_agi.c.patch
(42) res_clioriginate.c.patch
(43) res_convert.c.patch
(44) res_crypto.c.patch
(45) res_features.c.patch
(46) res_jabber.c.patch
(47) res_limit.c.patch
(48) res_musiconhold.c.patch
(49) res_odbc.c.patch
(50) res_realtime.c.patch
(51) RollUp.1.patch
(52) threadstorage.c.patch
(53) translate.c.patch
(54) udptl.c.patch
Description:Changed 'feature show' to 'features show' because it superposes with 'feature show channels' from chan_features (and has nothing to do with that and res_features is called featureSS so put an S on the CLI command too)
Comments:By: Sean Bright (seanbright) 2007-09-14 17:10:37

Added patch for res_agi.c
By: Sean Bright (seanbright) 2007-09-14 17:54:51

Added res_musiconhold.c (twice, apparently)
By: Moises Silva (moy) 2007-09-15 15:13:24

I opened 10732 for NEW_CLI changes in pbx.c, frame.c and logger.c, someone there suggested to keep al NEW_CLI in just 1 entry, given than eliel opened this one first I'm attaching my patches here, separated by file as well.

Can someone close 10732(33)? ( what a mess I did! )
By: Moises Silva (moy) 2007-09-16 18:40:13

added manager.c and http.c
By: Moises Silva (moy) 2007-09-17 08:58:09

added dnsmgr.c , In that file I registered a new cli_entry that was defined but not registered, don't know why ( dnsmgr refresh [pattern] ), I hope that's ok.
By: Eliel Sardanons (eliel) 2007-09-17 18:46:36

Added res/res_realtime.c patch against trunk rev 82729
By: Sean Bright (seanbright) 2007-09-17 19:49:23

Added res/res_odbc.c patch against trunk rev 82729
By: Eliel Sardanons (eliel) 2007-09-17 23:09:57

I recommend to commit this patches or this open issue will become a big and old patch and difficult to maintain against latest revision.
If you commit and close this issue we could open another to continue working on this janitor.
By: Eliel Sardanons (eliel) 2007-09-18 13:00:31

Added chan_sip.c :-) big patch.. against trunk rev 82729.
By: Sean Bright (seanbright) 2007-09-18 16:03:17

Added RollUp.1.patch which is a roll-up of all the previous 17 patches posted here against revision 82911 of trunk.
By: Digium Subversion (svnbot) 2007-09-18 17:25:00

Repository: asterisk
Revision: 82930

------------------------------------------------------------------------
r82930 | qwell | 2007-09-18 17:24:58 -0500 (Tue, 18 Sep 2007) | 24 lines

(issue ASTERISK-10302)
Reported by: eliel
Patches:
     res_features.c.patch uploaded by eliel (license 64)
     res_agi.c.patch uploaded by seanbright (license 71)
     res_musiconhold.c.patch uploaded by seanbright (license 71)
     pbx.c.patch uploaded by moy (license 222)
     logger.c.patch uploaded by moy (license 222)
     frame.c.patch uploaded by moy (license 222)
     manager.c.patch uploaded by moy (license 222)
     http.c.patch uploaded by moy (license 222)
     dnsmgr.c.patch uploaded by moy (license 222)
     res_realtime.c.patch uploaded by eliel (license 64)
     res_odbc.c.patch uploaded by seanbright (license 71)
     res_jabber.c.patch uploaded by eliel (license 64)
     chan_local.c.patch uploaded by eliel (license 64)
     chan_agent.c.patch uploaded by eliel (license 64)
     chan_alsa.c.patch uploaded by eliel (license 64)
     chan_features.c.patch uploaded by eliel (license 64)
     chan_sip.c.patch uploaded by eliel (license 64)
     RollUp.1.patch (includes all of the above patches) uploaded by seanbright (license 71)

Convert many CLI commands to the NEW_CLI format.

------------------------------------------------------------------------
By: Jason Parker (jparker) 2007-09-18 17:27:44

Committed all of the above patches.  I'm leaving this open in case there are still more (I'm sure there are).

(Note for future commits, karma has not yet been awarded - we should probably do that at the end)
By: Eliel Sardanons (eliel) 2007-09-18 22:15:41

added chan_iax2.c against trunk r83022.
By: Moises Silva (moy) 2007-09-19 09:04:23

added chan_zap.c

I could not test ss7 commands since I don't have ss7 available right now, but hey!, it compiles, ship it! ... seriously I don't think is an issue, ss7 had few commands and they were easy ones
By: Eliel Sardanons (eliel) 2007-09-19 10:11:24

Added app_queue.c against r83126, it also fix an issue on the autocomplete function of the 'queue remove member' CLI command.
By: Digium Subversion (svnbot) 2007-09-19 17:58:36

Repository: asterisk
Revision: 83213

------------------------------------------------------------------------
r83213 | qwell | 2007-09-19 17:58:36 -0500 (Wed, 19 Sep 2007) | 9 lines

More conversions to NEW_CLI

(issue ASTERISK-10302)
Patches:
     chan_zap.c.patch uploaded by moy (license 222)
     app_queue.c.patch uploaded by eliel (license 64)
     app_voicemail.c.patch uploaded by eliel (license 64)
     app_meetme.c.patch uploaded by eliel (license 64)

------------------------------------------------------------------------
By: Jason Parker (jparker) 2007-09-19 18:00:22

Please note, I did not commit the chan_iax2 patch.  There was an issue with __iax2_show_peers and casting.  char * and int are not the same on 64-bit machines.

I'll try to look at that one again tomorrow.
By: Moises Silva (moy) 2007-09-19 22:19:33

__iax2_show_peers return a char* but, only getting it from macros CLI_SUCCESS, CLI_SHOWUSAGE, CLI_FAILURE, those are defined like (char *)(int), so casting back  (int)(char *) is not really an issue. If you just want to get rid of the compile error/warning you can cast like this (int)(long)(char *)__iax2_show_peers()
By: Eliel Sardanons (eliel) 2007-09-20 07:49:57

#define RESULT_SUCCESS          0
#define RESULT_SHOWUSAGE        1
#define RESULT_FAILURE          2
#define CLI_SUCCESS     (char *)RESULT_SUCCESS
#define CLI_SHOWUSAGE   (char *)RESULT_SHOWUSAGE
#define CLI_FAILURE     (char *)RESULT_FAILURE
----------------
I did what I though was the way we change less code, I could find another solution if you don't like the 'cast' way.
By: Moises Silva (moy) 2007-09-20 08:57:29

added app_playback.c

eliel, I think you plan to keep working on this, what about doing a list of the missing files, possibly you starting from top -> down to the list, and I working down -> top, so we don't work on the same files?

I can do the list in the afternoon.
By: Eliel Sardanons (eliel) 2007-09-20 08:59:52

I agree, upload the list, and I will do it top->down.
By: Moises Silva (moy) 2007-09-20 12:38:09

List of missing files to update as of rev 83251 uploaded.
By: Jason Parker (jparker) 2007-09-20 13:07:57

I have a few comments about some of the patches I've seen.

Sometimes there is a "switch ( cmd ) {", which should be "switch (cmd) {" (note the spacing).

There are a few places where I've seen "argc > 3" converted to "3 < argc" - in my opinion, it's more difficult to read like that.  Most other places in asterisk use the former.

Also, a few functions have had int fd, argc and char *argv[] declared in the function, rather than using a->fd or a->argc - this should be avoided.  I'd much rather see any places that uses fd changed to use a->fd


Other than that, these are great - keep them coming, and I'll keep committing.
By: Eliel Sardanons (eliel) 2007-09-20 13:29:30

qwell: "Also, a few functions have had int fd, argc and char *argv[] declared in the function, rather than using a->fd or a->argc - this should be avoided. I'd much rather see any places that uses fd changed to use a->fd"

Please can you give me an example of this, I couldn't understand you.

Thanks
By: Jason Parker (jparker) 2007-09-20 13:33:07

int fd = a->fd, argc = a->argc;
char **argv = a->argv;

ast_cli(fd, "Blah\n");

Rather than just

ast_cli(a->fd, "Blah\n");


The latter is what it should be, and not the former.


By: Eliel Sardanons (eliel) 2007-09-20 13:37:09

moy: Sorry for astmm.c it was done before the list was commited.
Notice the change made for the handler function:
From handle_show_memory to handle_memory_show making more sense with the new cli command name we should try to change this too.
By: Eliel Sardanons (eliel) 2007-09-20 13:47:48

Thanks qwell, now I got it.
By: Digium Subversion (svnbot) 2007-09-20 17:56:12

Repository: asterisk
Revision: 83381

------------------------------------------------------------------------
r83381 | qwell | 2007-09-20 17:56:12 -0500 (Thu, 20 Sep 2007) | 8 lines

More NEW_CLI conversions.

(issue ASTERISK-10302)
Patches:
     app_playback.c.patch uploaded by moy (license 222)
     app_minivm.c.patch uploaded by eliel (license 64)
     astmm.c.patch uploaded by eliel (license 64)

------------------------------------------------------------------------
By: Eliel Sardanons (eliel) 2007-09-20 18:24:48

Qwell, which is the future of the chan_iax2.c.patch? should I change the way we do the return CLI_* without a cast?
By: Eliel Sardanons (eliel) 2007-09-20 21:05:18

Please delete chan_mgcp.c.patch and use chan_mgcp.c.patch2
chan_mgcp has ugly code, using the handler for the 'mgcp reload' cli command for the reload_module(), and deprecating the 'mgcp reload' (dont know why) in a rare way.
By: Sean Bright (seanbright) 2007-09-20 21:11:27

I continued from the middle of the list :-)
By: Eliel Sardanons (eliel) 2007-09-20 21:19:59

Added iax2-provision.c:
      Solved a trivial issue with the autocomplete.
      Made a text correction for the cli command usage.
By: Moises Silva (moy) 2007-09-20 22:12:25

eliel: never mind about astmm.c :-) ... I did not start anyway

seanbright: welcome back to this bug !

added cli.c and astobj2.c

I would like to some one else give a check to cli.c, after moving to NEW_CLI some internal "_command" entries are not listed anymore in the console. Everything seems to work though.
By: Moises Silva (moy) 2007-09-20 23:30:17

added asterisk.c ... good night to everyone ...
By: Digium Subversion (svnbot) 2007-09-21 07:00:48

Repository: asterisk
Revision: 83399

------------------------------------------------------------------------
r83399 | simon.perreault | 2007-09-21 07:00:43 -0500 (Fri, 21 Sep 2007) | 152 lines

Merged revisions 83229,83231,83233-83234,83251,83278,83293-83298,83317,83349-83351,83381 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
r83229 | file | 2007-09-20 12:10:57 -0400 (Thu, 20 Sep 2007) | 2 lines

Fix memory leaks in pbx_dundi, cdr_pgsql, and the configuration file parser.

................
r83231 | file | 2007-09-20 12:19:45 -0400 (Thu, 20 Sep 2007) | 15 lines

Merged revisions 83230 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r83230 | file | 2007-09-20 13:17:24 -0300 (Thu, 20 Sep 2007) | 7 lines

Fix a minor spelling error.
(closes issue ASTERISK-10343)
Reported by: flefoll
Patches:
     chan_sip.c.trunk.83071.inita-patch uploaded by flefoll (license 244)
     chan_sip.c.br14.83070.inita-patch uploaded by flefoll (license 244)

........

................
r83233 | russell | 2007-09-20 12:27:07 -0400 (Thu, 20 Sep 2007) | 3 lines

Don't start the event processing thread until after forking.
(reported by Simon on the -dev list, thanks!)

................
r83234 | file | 2007-09-20 12:28:00 -0400 (Thu, 20 Sep 2007) | 15 lines

Merged revisions 83232 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r83232 | file | 2007-09-20 13:25:30 -0300 (Thu, 20 Sep 2007) | 7 lines

Make sure the minimum T1 timer value is obeyed in all cases.
(closes issue ASTERISK-10342)
Reported by: flefoll
Patches:
     chan_sip.c.trunk.83071.retrans-patch uploaded by flefoll (license 244)
     chan_sip.c.br14.83070.retrans-patch uploaded by flefoll (license 244)

........

................
r83251 | qwell | 2007-09-20 13:10:14 -0400 (Thu, 20 Sep 2007) | 16 lines

Merged revisions 83246 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r83246 | qwell | 2007-09-20 12:09:14 -0500 (Thu, 20 Sep 2007) | 8 lines

If # is pressed after dialing an extension in DISA, stop trying to collect more digits.

(closes issue ASTERISK-10329)
Reported by: atis
Patches:
     app_disa.c.branch.patch uploaded by atis (license 242)
     app_disa.c.trunk.patch uploaded by atis (license 242)

........

................
r83278 | qwell | 2007-09-20 15:05:16 -0400 (Thu, 20 Sep 2007) | 1 line

Fix a trivial typo, to test our new commit bot
................
r83293 | russell | 2007-09-20 15:11:31 -0400 (Thu, 20 Sep 2007) | 1 line

trivial formatting change
................
r83294 | russell | 2007-09-20 15:17:16 -0400 (Thu, 20 Sep 2007) | 1 line

fix spelling in a comment
................
r83295 | russell | 2007-09-20 15:22:10 -0400 (Thu, 20 Sep 2007) | 1 line

minor grammar fix
................
r83296 | russell | 2007-09-20 15:32:18 -0400 (Thu, 20 Sep 2007) | 1 line

minor spelling fixes in a comment
................
r83297 | russell | 2007-09-20 15:42:33 -0400 (Thu, 20 Sep 2007) | 1 line

trivial formatting change
................
r83298 | russell | 2007-09-20 15:45:00 -0400 (Thu, 20 Sep 2007) | 1 line

trivial formatting change
................
r83317 | russell | 2007-09-20 17:02:17 -0400 (Thu, 20 Sep 2007) | 11 lines

Merged revisions 83316 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r83316 | russell | 2007-09-20 16:01:20 -0500 (Thu, 20 Sep 2007) | 3 lines

Change safe_asterisk to explicitly ask for /bin/bash, as it uses bashisms.
(closes issue ASTERISK-10345, reported by culrich)

........

................
r83349 | russell | 2007-09-20 17:17:39 -0400 (Thu, 20 Sep 2007) | 12 lines

Merged revisions 83348 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r83348 | russell | 2007-09-20 16:16:48 -0500 (Thu, 20 Sep 2007) | 4 lines

When daemonizing, don't change working directory to "/".  It makes it not be
able to do a core dump when not running as uid=root.
(closes issue ASTERISK-10340, xrg)

........

................
r83350 | mmichelson | 2007-09-20 17:21:28 -0400 (Thu, 20 Sep 2007) | 3 lines

Merging changes from queue_refcount_trunk into trunk. Refcounted queues now in place.


................
r83351 | mmichelson | 2007-09-20 17:37:28 -0400 (Thu, 20 Sep 2007) | 4 lines

Oops. Getting rid of svnmerge-integrated and automerge stuff



................
r83381 | qwell | 2007-09-20 19:14:30 -0400 (Thu, 20 Sep 2007) | 8 lines

More NEW_CLI conversions.

(issue ASTERISK-10302)
Patches:
     app_playback.c.patch uploaded by moy (license 222)
     app_minivm.c.patch uploaded by eliel (license 64)
     astmm.c.patch uploaded by eliel (license 64)

................

------------------------------------------------------------------------
By: Sean Bright (seanbright) 2007-09-21 15:01:21

NOTE:  app_osplookup.c.patch is completely untested.  If someone wants to test it before applying, that would be great.
By: Moises Silva (moy) 2007-09-22 12:57:23

added cdr.c and pbx_dundi ( typo, file is named pbd_dundi.c.patch )
By: Moises Silva (moy) 2007-09-22 13:44:50

seanbright: I have uploaded app_osplookup-rev83558.patch, since the one you did it did not compile :( . After a minor typo fix, removing extra curly braces and adding curly braces that were removed but caused ANSI warnings, it worked like a charm :)
By: Sean Bright (seanbright) 2007-09-22 14:06:13

Cool.  Like I said in my note, I wasn't able to test it at all since I didn't have OSP installed.
By: Sean Bright (seanbright) 2007-09-27 08:39:32

*cough* bump *cough*
By: Eliel Sardanons (eliel) 2007-09-27 08:43:12

There is an issue related to this janitor: ASTERISK-1077831
My mistake.
By: Adam Goryachev (adamg) 2007-10-03 18:14:48

I don't think this is related to this issue, except for the comment from:
(0070796)
eliel - reporter
09-19-07 10:11
Added app_queue.c against r83126, it also fix an issue on the autocomplete function of the 'queue remove member' CLI command.

I just updated to current SVN last night, and this morning found this problem. From the CLI if I type "queue remove member <TAB>" then asterisk will crash. I've reproduced this twice.

queue remove member *** glibc detected *** free(): invalid pointer: 0x08247d70 ***
Asterisk ended with exit status 0

There was nothing written to the log files but there is a core dump.
I am using version SVN-branch-1.4-r84474M
I am using debian stable with all updates
I tried to read the guidelines to find out what to do with the core file, but it simply said to "see the notes below" but there were no notes below on gdb.... (the test gdb only appears once on the page).

Anyway, hopefully this is a simple fix, but if you need additional info, let me know and I will re-test or provide the info needed.

Regards,
Adam
By: Moises Silva (moy) 2007-10-03 20:56:42

adamg: Update your trunk version, I am not able to reproduce, so I suppose someone else fixed that
By: Adam Goryachev (adamg) 2007-10-04 01:13:18

I've just updated to latest SVN (1.4 branch) SVN-branch-1.4-r84581M and I no longer see the glibc error, but I do still get a asterisk crash. Also, I didn't see any updates to the cli files/etc. I also got another core file.

Please advise what info I should provide from the core file to be more helpful.

Thanks Adam
By: Eliel Sardanons (eliel) 2007-10-04 07:47:25

adamg: Please open a new bug report and upload a 'bt full'.
By: Digium Subversion (svnbot) 2007-10-11 13:42:36

Repository: asterisk
Revision: 85460

U   trunk/apps/app_mixmonitor.c
U   trunk/apps/app_osplookup.c
U   trunk/apps/app_rpt.c
U   trunk/channels/chan_gtalk.c
U   trunk/channels/chan_mgcp.c
U   trunk/channels/chan_oss.c
U   trunk/channels/chan_skinny.c
U   trunk/channels/iax2-provision.c
U   trunk/main/asterisk.c
U   trunk/main/astobj2.c
U   trunk/main/cdr.c
U   trunk/main/channel.c
U   trunk/main/cli.c
U   trunk/main/db.c
U   trunk/main/file.c
U   trunk/main/image.c
U   trunk/main/threadstorage.c
U   trunk/main/translate.c
U   trunk/main/udptl.c
U   trunk/pbx/pbx_ael.c
U   trunk/pbx/pbx_config.c
U   trunk/pbx/pbx_dundi.c
U   trunk/res/res_clioriginate.c
U   trunk/res/res_convert.c
U   trunk/res/res_crypto.c
U   trunk/res/res_limit.c

------------------------------------------------------------------------
r85460 | russell | 2007-10-11 13:42:35 -0500 (Thu, 11 Oct 2007) | 33 lines

Merge a ton of NEW_CLI conversions.  Thanks to everyone that helped out!  :)

(closes issue ASTERISK-10302)
Reported by: eliel
Patches:
     chan_skinny.c.patch uploaded by eliel (license 64)
     chan_oss.c.patch uploaded by eliel (license 64)
     chan_mgcp.c.patch2 uploaded by eliel (license 64)
     pbx_config.c.patch uploaded by seanbright (license 71)
     iax2-provision.c.patch uploaded by eliel (license 64)
     chan_gtalk.c.patch uploaded by eliel (license 64)
     pbx_ael.c.patch uploaded by seanbright (license 71)
     file.c.patch uploaded by seanbright (license 71)
     image.c.patch uploaded by seanbright (license 71)
     cli.c.patch uploaded by moy (license 222)
     astobj2.c.patch uploaded by moy (license 222)
     asterisk.c.patch uploaded by moy (license 222)
     res_limit.c.patch uploaded by seanbright (license 71)
     res_convert.c.patch uploaded by seanbright (license 71)
     res_crypto.c.patch uploaded by seanbright (license 71)
     app_osplookup.c.patch uploaded by seanbright (license 71)
     app_rpt.c.patch uploaded by seanbright (license 71)
     app_mixmonitor.c.patch uploaded by seanbright (license 71)
     channel.c.patch uploaded by seanbright (license 71)
     translate.c.patch uploaded by seanbright (license 71)
     udptl.c.patch uploaded by seanbright (license 71)
     threadstorage.c.patch uploaded by seanbright (license 71)
     db.c.patch uploaded by seanbright (license 71)
     cdr.c.patch uploaded by moy (license 222)
     pbd_dundi.c.patch uploaded by moy (license 222)
     app_osplookup-rev83558.patch uploaded by moy (license 222)
     res_clioriginate.c.patch uploaded by moy (license 222)

------------------------------------------------------------------------
By: Russell Bryant (russell) 2007-10-15 08:07:33

I'm reopening this because I need some help from you guys looking over all of these patches.  Apparently, tab completion got removed by some of these changes.  The one specific one that I know of is "sip show peers".  There may be others, though.  We need to make sure that if a CLI command had tab completion before, that it got moved to the new command, and not removed.
By: Russell Bryant (russell) 2007-10-15 08:13:23

Nevermind, I meant "sip show peer", and that one has it ... Stay tuned while I figure out exactly what the problem is about tab completion people are reporting.  :)
By: Russell Bryant (russell) 2007-10-15 08:24:59

false alarm.  10970 fixed it all up ...