[Home]

Summary:ASTERISK-06560: [patch] Allow MeetMe to use ',' deliminators in meetme.conf
Reporter:Mark Monnin (wrmem)Labels:
Date Opened:2006-03-16 15:47:45.000-0600Date Closed:2006-03-21 00:04:29.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_meetme
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20060317__bug6738.diff.txt
( 1) meetme-patch-v2.txt
( 2) meetme-patch-v3.txt
Description:After MeetMe was converted to use the new argument macros, it no longer accepts commas to define static conferences.  In the example given in "additional information", conference 37404 would work, and 37403 would not.

Comma's are what is used in the sample "configs/meetme.conf.sample"

I also made a small clean up to the logic in handling empty pins and empty adminpins.  Same functionality as before, but more readable (hopefully).

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

Disclaimer on file.

meetme.conf
------------------
[rooms]
conf => 37403,1212
conf => 37404|3434

extensions.conf
-------------------
[meetme-context]
exten => 37000,1,MeetMe()
exten => 37000,n,Hangup


Comments:By: Clod Patry (junky) 2006-03-16 17:07:32.000-0600

as asked on IRC.
By: Olle Johansson (oej) 2006-03-17 03:10:48.000-0600

+ if (!args.pin) { args.pin = ""; }
+ if (!args.pinadmin) { args.pinadmin = ""; }

You need a line break after the if - check the coding guidelines. ;-)
By: Mark Monnin (wrmem) 2006-03-17 09:47:18.000-0600

Updated version.  Hopefully more CODELINE compliant.  (With slightly even more Doxygen to make oej happy).
By: Tilghman Lesher (tilghman) 2006-03-17 10:28:15.000-0600

Still not compliant with the coding guidelines.  Specifically:

1.  The } at the end of a function should be in column 1.

2.  Some spelling corrections:  "delimiter", "separate", "parentheses".

3.  Change your EXTSTANDARD to MULTI_STANDARD.

4.  You can simplify the build_conf logic to:
cnf = build_conf(args.confno, ast_strlen_zero(args.pin) ? "" : args.pin, ast_strlen_zero(args.pinadmin) ? "" : args.pinadmin, make, dynamic, refcount);

5.  Also, I don't see the addition of this code as either necessary or desireable.  Typically, if we're parsing comma-delimited arguments, we want to preserve the vertical-bar-delimiters (as they are a separate grouping of arguments).  What you should probably be using is the NONSTANDARD version of the app args macro, with a simple comma delimiter.

Due to 5, I'm going to fail this patch for architectural reasons, as well.
By: Mark Monnin (wrmem) 2006-03-17 10:41:36.000-0600

Ok on #1..#4.

Not sure on ASTERISK-1.  In meetme.conf.sample, the example shows up as comma seperated.  It appears that verticle bar seperated is ok too, so I was trying to handle both cases.  (See closed bug 3646, http://bugs.digium.com/view.php?id=3646)

The patch doesn't preclude a comma seperated only parsing for other applications, but that's not what MeetMe appears to need (unless we want to force only one format).
By: Tilghman Lesher (tilghman) 2006-03-17 10:46:15.000-0600

You could do what I've done in the SORT function; that is, look for the occurrence of a comma and use ONLY a comma as a delimiter if you find it, or fallback to using '|'.  However, using both at the same time is not desireable.
By: Mark Monnin (wrmem) 2006-03-20 16:44:01.000-0600

(Corydon76 - your version didn't appear to handle the vertical bar case.)

I've uploaded meetme-patch-v3.txt.  It will now try to split on vertical bars first, then if it did not split the value, it will try again with comma's.

I've also reworked the refactoring of build_conf's args (and put the logic inside of the subroutine).  The other refactoring (ast_strlen_zero) was from Corydon76's patch.
By: Tilghman Lesher (tilghman) 2006-03-20 17:42:13.000-0600

Your patch is now backwards.  It should prefer to use comma as a delimiter, and only FALLBACK to using a vertical bar.
By: Tilghman Lesher (tilghman) 2006-03-20 17:43:17.000-0600

Actually, now that I think of it, it should NEVER handle the vertical bar case.  This is not documented behavior and we should not encourage bad syntax in the file.  Vertical bar is simply wrong.
By: Tilghman Lesher (tilghman) 2006-03-21 00:04:29.000-0600

Committed my own patch to trunk