[Home]

Summary:ASTERISK-03590: app_system additions.
Reporter:Matt O'Gorman (mogorman)Labels:
Date Opened:2005-02-25 23:21:55.000-0600Date Closed:2008-01-15 15:26:23.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_system
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_readfile.c
( 1) patch
( 2) patch
( 3) patch
( 4) patch
Description:Adds ability for app_system to take in output of command, stores it in temp file, reads into asterisk, and then deletes it.

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

Disclaimer on file
Comments:By: Mark Spencer (markster) 2005-02-26 00:23:10.000-0600

Please attach in cvs diff -u format.  Does this replace the other app?
By: Matt O'Gorman (mogorman) 2005-02-26 08:15:53.000-0600

yes it replaces the other app_readpipe.
By: Mark Spencer (markster) 2005-02-26 10:42:13.000-0600

Okay, great!  Now just a few things to clear up:

1) When you execute malloc() realloc() in ast_read_file, you have to check its return value to be sure it gracefully handles running out of memory.  Note that if realloc returns NULL it means the reallocation was not successful, but you would still need to free the block you had.  Another option to malloc/realloc would be to use stat (see man 2 stat) to read the size of the file before hand.  You would still want to be sure you didn't overwrite the end of your buffer in case someone magically added stuff to the file in between stat() and your reading of the file, but generally then you could just have the one malloc and not have to worry about the realloc calls.

2) (notwithstanding item 3) You should be using snprintf into a suitably large buffer instead of all those strcat's, it will make things much simpler.

3) It presumably makes more sense having this read file functionality as a separate application (Say "ReadFile") rather than building it into system since people might want to read a file without system() being executed.  Doing this also eliminates the need to construct the command line for the user.

Thanks!
By: Matt O'Gorman (mogorman) 2005-02-26 12:31:31.000-0600

thanks for suggestions, and have written app_readfile, just not quite done with it  yet, I will look into other suggestions.
By: Matt O'Gorman (mogorman) 2005-02-27 01:35:40.000-0600

Okay here it is with updates, app_readfile takes in (var=filename,length) where length is length in charcters, same as app_system now.  However for system to read in a file you need to have /var/spool/asterisk/system directory to exist, I don't know how to add that to the makefile as I am ignorant.
By: Matt O'Gorman (mogorman) 2005-02-27 02:38:48.000-0600

ahh there it is. ^_^ newest patch.

Also something I noticed while doing this noop does not like really big variables, variables bigger than 3950 or so.  It deadlocks the channel, or at least I think it does.  Here is what happens, you input a very large file, the import comes in.  And the channel will come up and down with the huge varible, but if you do a noop the channel "dissapears" it isn't usable again, nor is it visible.  Thoughts?
By: Mark Spencer (markster) 2005-02-27 13:10:18.000-0600

Added to CVS with a few changes:

1) Changed function name to ast_read_textfile since we already have ast_readfile and having ast_readfile and ast_read_file doing totally different things would seem to potentially lead to confusion.

2) Changed char * to const char * where appropriate.

3) Removed unnecessary parameter "mode" from function since, if you're reading, it would always be "r".

4) Simplified by using unbuffered I/O to read in the file.

5) Removed duplicate malloc of memory.

6) Cleaned up the app a little bit.  

7) Removed changes to system since you can use System and then ReadFile to obtain the same thing.

8) Confirmed that on my box it reads a 20k file okay.

9) Cleaned up copyright header.

Thanks!
By: Russell Bryant (russell) 2005-02-27 18:36:44.000-0600

not included in 1.0 since it is a new feature
By: Digium Subversion (svnbot) 2008-01-15 15:26:23.000-0600

Repository: asterisk
Revision: 5099

U   trunk/Makefile
U   trunk/app.c
U   trunk/apps/Makefile
A   trunk/apps/app_readfile.c
U   trunk/include/asterisk/app.h

------------------------------------------------------------------------
r5099 | markster | 2008-01-15 15:26:22 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge mog's ReadFile application (bug ASTERISK-3590)

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

http://svn.digium.com/view/asterisk?view=rev&revision=5099