[Home]

Summary:ASTERISK-15329: [patch] for reading and writing to text file
Reporter:skyman (skyman)Labels:
Date Opened:2009-12-17 09:55:48.000-0600Date Closed:2010-07-13 13:31:42
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20100622__issue16461.diff.txt
( 1) func-env.patch
( 2) func-env-1.patch
( 3) func-env-2.patch
( 4) func-env-3.patch
( 5) func-env-4.patch
( 6) func-env-5.patch
( 7) test_func_file.c
Description:this patch adds 7 functions in func_env :
FILE_FORMAT
FILE_COUNT_LINE
FILE_READ_LINE
FILE_WRITE
FILE_WRITE_LINE
FILE_APPEND
FILE_APPEND_LINE
Comments:By: skyman (skyman) 2010-01-14 08:40:20.000-0600

The WRITE functions are intended to replace :
system(echo "string" > file.txt)
used by many users, by :
FILE_WRITE("file.txt","string")

The READ functions are used to read text files, written by a web application for example.
By: Russell Bryant (russell) 2010-05-18 14:13:23

This patch needs to be made against Asterisk trunk.  The biggest change is that the documentation will have to be in the XML format.
By: John Todd (jtodd) 2010-05-18 14:13:52

Pretty useful; this would have made quite a few of my configs in the past a bit more elegant and unified.
By: skyman (skyman) 2010-05-20 08:26:40

New patch uploaded for Asterisk 1.6.2 : func-env-2.patch
No change, just added documentation in XML format.
By: Paul Belanger (pabelanger) 2010-05-20 08:43:32

skyman: Thanks for the patch. There are a few problems with your formatting (see CODING GUIDELINES). It might we worth it to post this to reviewboard to get more eyes on it.
By: skyman (skyman) 2010-05-25 04:37:36

New patch uploaded : func-env-3.patch
correction code formatting
By: Tilghman Lesher (tilghman) 2010-05-25 12:42:36

I'd much rather you add functionality to the FILE() dialplan function and use parameters to the dialplan function to specify the differences.  In addition, you're completely ignoring the write portion of dialplan functions.  Also, the .read portion of the API is deprecated.  You should instead be creating .read2 portions of the API, as this method of handling the buffer is much more efficient.
By: skyman (skyman) 2010-05-26 08:15:03

You said :
"Also, the .read portion of the API is deprecated."
you mean the "file_read" is deprecated ?
if that's it, I can change the function "file_read" without asking any question about backward compatibility?
I'm not sure this is a good idea. I prefer to keep backward compatibility.


You said :
"You should instead be creating .read2 portions of the API, as this method of
handling the buffer is much more efficient."
I dont understand. Have you some documentation or example of that ?


You said :
"I'd much rather you add functionality to the FILE() dialplan function and use parameters to the dialplan function to specify the differences.
In addition, you're completely ignoring the write portion of dialplan functions."
I know the ".write" portion of the dialplan, but that requires changing the ".read" as.

I think a solution like this :
replace the :
FILE(filename,offset,length)
by
FILE(filename,offset,length[,readline[,format]])

[Arguments]
offset
   Maybe specified as any number. If negative, <offset> specifies the
   number of bytes back from the end of the file.
length
   If specified, will limit the length of the data read to that size.
   If negative, trims <length> bytes from the end of the file.

readline
   If set to 1, read one line at a time. <offset> specifie the line to be read, <length> will be ignored.

format
   Format may be one of the following:
       u - Unix newline format (default).
d - Dos newline format.
m - Mac newline format.

examples :
FILE(/tmp/test.txt,10,20) : reads the 10th to 19th character in the file. (for backware compatibility)
FILE(/tmp/test.txt,3,,1) : reads the 3th line of the file.
FILE(/tmp/test.txt,3,,1,d) : reads the 3th line of file in DOS format.

This preserves backward compatibility, but I'm not sure this is a good idea.
What is your opinion ?

(sorry for my bad english, i am french)
By: Tilghman Lesher (tilghman) 2010-05-26 09:52:56

The read2 API call uses an ast_str dynamic buffer structure for the result, instead of relying on a static buffer passed into the function.  This allows the buffer to start out much smaller and to grow as necessary, which is more memory-efficient for the common case, but also allows much larger values to be processed in the exceptional case.

My suggestion for the arguments would be:
FILE(name,[L]offset,[L]length,file-format),
such that prefixing the offset with the 'L' character specifies the offset is to be interpreted as a line number, and that prefixing the length argument with an 'L' character specifies the number of lines to be read.  I agree with your specification of the file-format, though, possibly, the default should agree with the platform (i.e. on Cygwin, the default should be 'dos').
By: skyman (skyman) 2010-05-26 11:21:00

I change the "file-format" to reflect the system on which it will be compiled.

For the ".read2", i do like that ?
static struct ast_custom_function file_function = {
.name = "FILE",
.read2 = file_read
.write = file_write
};


New description of FILE function :

<function name="FILE" language="en_US">
<synopsis>
Read or write file.
</synopsis>
<syntax>
<parameter name="filename" required="true" />
<parameter name="offset" required="true">
<para>reading mode</para>
<para>Maybe specified as any number. If negative, <replaceable>offset</replaceable> specifies the number
of bytes back from the end of the file.</para>
<para>if the offset is prefixing with the 'L' character, the offset is to be interpreted as a line number</para>
<para>writing mode</para>
<para>Offset may be one of the following :</para>
<para>w - write (all the previous content will be deleted).</para>
<para>a - the string will be added to the end of file.</para>
</parameter>
<parameter name="length" required="true">
<para>reading mode</para>
<para>If specified, will limit the length of the data read to that size. If negative,
trims <replaceable>length</replaceable> bytes from the end of the file.</para>
<para>if the length is prefixing with the 'L' character, the offset is to be interpreted as a number of lines to be read</para>
<para>writing mode</para>
<para>Length may be one of the following :</para>
<para>0 - write the string only.</para>
<para>1 - write the string and append a newline.</para>
</parameter>
<parameter name="format">
<para>Format may be one of the following :</para>
<para>u - Unix newline format (default except for CYGWIN).</para>
<para>d - Dos newline format (default for CYGWIN).</para>
<para>m - Mac newline format</para>
</parameter>
</syntax>
<description>
<para>Example for read mode :</para>
<para>;reads the 10th to 19th character in the file.</para>
<para>exten => s,1,Set(foo=${FILE(/tmp/test.txt,10,20)})</para>
<para>; reads the 3th line of the file.</para>
<para>exten => s,1,Set(foo=${FILE(/tmp/test.txt,L3,L1)})</para>
<para>; reads the 3th and 4th line of the file.</para>
<para>exten => s,1,Set(foo=${FILE(/tmp/test.txt,L3,L2)})</para>
<para>; reads the 3th line of a DOS formated file.</para>
<para>exten => s,1,Set(foo=${FILE(/tmp/test.txt,L3,L1,d)})</para>
<para></para>
<para>Example for write mode</para>
<para>; append "test" to the file.</para>
<para>exten => s,1,Set(FILE(/tmp/test.txt,a,0)=test)</para>
<para>; append "test" and a Unix newline to the file.</para>
<para>exten => s,1,Set(FILE(/tmp/test.txt,a,1)=test)</para>
<para>; append "test" and a DOS newline to the file.</para>
<para>exten => s,1,Set(FILE(/tmp/test.txt,a,1,d)=test)</para>
<para></para>
<para>; erase all the content of the file and write "test" to the file.</para>
<para>exten => s,1,Set(FILE(/tmp/test.txt,w,0)=test)</para>
<para>; erase all the content of the file and write "test" with a Unix (DOS for CYGWIN) newline to the file.</para>
<para>exten => s,1,Set(FILE(/tmp/test.txt,w,1)=test)</para>
<para>; erase all the content of the file and write "test" with a DOS newline to the file.</para>
<para>exten => s,1,Set(FILE(/tmp/test.txt,w,1,d)=test)</para>
</description>
</function>

It's ok like this ?

Thanks for your help.
By: Tilghman Lesher (tilghman) 2010-05-26 11:36:35

Actually, I don't think we need to alter the meaning of the parameters for write:

; Replace the first line of the file with "bar"
Set(FILE(/tmp/foo.txt,L1,L1)=bar)
; Replace the last line of the file with "bar"
Set(FILE(/tmp/foo.txt,L-1,L1)=bar)
; Append "bar" to the file
Set(FILE(/tmp/foo.txt,L0)=bar)
; Tricky: truncate the file and write "bar" to the file (simply, replace lines 1-999999 with "bar")
Set9FILE(/tmp/foo.txt,L1,L999999)=bar)
By: skyman (skyman) 2010-05-27 04:48:37

I have not done the "replace" just "write" and "append". I change to the "replace".

I propose that :

Read mode (char) :
------------------
;reads the the entire content of the file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt)})

;reads the 10th to end of file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt,10)})

;reads the 10th to 19th character in the file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt,10,20)})


Read mode (Line) :
------------------
; reads the 3th line of the file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt,L3,L1)})

; reads the 3th and 4th line of the file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt,L3,L2)})

; reads all lines of the file from the third .
exten => s,1,Set(foo=${FILE(/tmp/test.txt,L3)})

; reads the last three lines  of the file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt,L-1,L3)})


Replace mode (char) :
---------------------
; Replace the first char of the file with "bar"
Set(FILE(/tmp/foo.txt,1,1)=bar)

; Replace 10 chars begining at the 19th of the file with "bar"
Set(FILE(/tmp/foo.txt,19,10)=bar)

; Replace all chars from the 19th with "bar"
Set(FILE(/tmp/foo.txt,19,-1)=bar)

; Append "bar"
Set(FILE(/tmp/foo.txt,-1)=bar)


Replace mode (Line) :
---------------------
; Replace the first line of the file with "bar"
Set(FILE(/tmp/foo.txt,L1,L1)=bar)

; Replace the last line of the file with "bar"
Set(FILE(/tmp/foo.txt,L-1,L1)=bar)

; Append "bar" to the file + a newline
Set(FILE(/tmp/foo.txt,L0)=bar)
I think it's better like that :
Set(FILE(/tmp/foo.txt,L-1)=bar)
and/or
Set(FILE(/tmp/foo.txt,L-1,L0)=bar)


for the last example
; Tricky: truncate the file and write "bar" to the file (simply, replace
lines 1-999999 with "bar")
Set(FILE(/tmp/foo.txt,L1,L999999)=bar)
I think it's better like that :
Set(FILE(/tmp/foo.txt,L1,L-1)=bar)
and/or
Set(FILE(/tmp/foo.txt,L0,L-1)=bar)

your opinion ?
By: Tilghman Lesher (tilghman) 2010-05-27 10:34:35

These examples are commented incorrectly (reason in parentheses):

;reads the 11th to end of file (0-based offsets).
exten => s,1,Set(foo=${FILE(/tmp/test.txt,10)})

;reads the 11th to 30th character in the file (10+20).
exten => s,1,Set(foo=${FILE(/tmp/test.txt,10,20)})

; Replace 10 chars beginning at the 20th of the file with "bar" (0-based offsets)
Set(FILE(/tmp/foo.txt,19,10)=bar)

; Replace all chars from the 20th, up until the second to last with "bar" (0-based offsets)
Set(FILE(/tmp/foo.txt,19,-1)=bar)

To do this, I would recommend the following syntax:
; reads the last three lines of the file.
exten => s,1,Set(foo=${FILE(/tmp/test.txt,L-3)})

Negative numbers generally mean "take back from the end".  See variable substitution, which does this already.

I just realized that truncating the file can actually be written much simpler:
Set(FILE(/tmp/foo.txt)=bar)

I stand by my suggestion that appending to the file use the L0 line specification, or if you think it's more clear, L-0 (take back 0 lines from the end).
By: skyman (skyman) 2010-05-31 06:19:37

Sorry, i write comments to quicly
But, I suggest another approach :

;reads the 10th to end of file (1-based offsets, 9th to end in 0-based).
exten => s,1,Set(foo=${FILE(/tmp/test.txt,10)})

;reads the 10th to 29th character in the file (9 to 28 0-based).
exten => s,1,Set(foo=${FILE(/tmp/test.txt,10,20)})

; Replace 10 chars beginning at the 19th of the file with "bar" (1-based
offsets)
Set(FILE(/tmp/foo.txt,19,10)=bar)

; Replace all chars from the 19th to last with "bar"
(1-based offsets)
Set(FILE(/tmp/foo.txt,19,-1)=bar)


I stand by my suggestion that appending to the file use the L0 line
specification, or if you think it's more clear, L-0 (take back 0 lines from
the end).

I prefer to keep L-1 for add a line :
exten => s,1,Set(${FILE(/tmp/test.txt,L-1)}=bar)

And to add a string :
exten => s,1,Set(${FILE(/tmp/test.txt,-1)}=bar)


I reserved the '0' for 'begin of file' :
; truncate the file and write 'bar'
Set(FILE(/tmp/foo.txt)=bar)

; equivalent to
Set(FILE(/tmp/foo.txt,L0)=bar)
By: skyman (skyman) 2010-06-08 08:50:41

New patch uploaded : func-env-4.patch
By: Tilghman Lesher (tilghman) 2010-06-08 10:00:27

Have you run all of the examples in the documentation and verified that they function as advertised?
By: skyman (skyman) 2010-06-17 05:20:56

New patch uploaded : func-env-5.patch

some Warning changed to error
best support of negative offset
adding example of creating empty file
bug fixing
all examples were tested

in this patch, only 2 functions added :
FILE_FORMAT
FILE_COUNT_LINE

Modification of FILE function
Adding write mode
Adding line mode for reading and writing
By: Tilghman Lesher (tilghman) 2010-06-17 19:35:58

I have created a test module which contains a number of tests, all of which I would expect to succeed.  Even some of the character-based read tests are failing, so a lot more debugging still needs to be done.
By: Tilghman Lesher (tilghman) 2010-06-22 23:35:24

I have altered the syntax slightly, added a ton more tests, coded the FILE() function from scratch, and verified that the tests succeed against the new code.
By: Digium Subversion (svnbot) 2010-07-13 13:31:40

Repository: asterisk
Revision: 276114

U   trunk/CHANGES
U   trunk/funcs/func_env.c
A   trunk/tests/test_func_file.c

------------------------------------------------------------------------
r276114 | tilghman | 2010-07-13 13:31:40 -0500 (Tue, 13 Jul 2010) | 10 lines

FILE() now supports line-mode and writing (altering) files.

(closes issue ASTERISK-15329)
Reported by: skyman
Patches:
      20100622__issue16461.diff.txt uploaded by tilghman (license 14)
Tested by: tilghman

Review: https://reviewboard.asterisk.org/r/737/

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

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