> On Sept. 30, 2014, 1:37 p.m., Mark Michelson wrote: > > branches/12/main/config.c, lines 751-753 > > <https://reviewboard.asterisk.org/r/4033/diff/2/?file=67696#file67696line751> > > > > This if allows for a zero-length category_name to result in a true > > return from does_category_match(). This is especially easy to do in the > > case that a NULL match parameter is passed in. > > > > I think the if should be rewritten as: > > > > if (ast_strlen_zero(category_name) || strcasecmp(cat->name, > > category_name))
That was the expected behavior. I'll add some comments to explain. > On Sept. 30, 2014, 1:37 p.m., Mark Michelson wrote: > > branches/12/main/config.c, lines 795-808 > > <https://reviewboard.asterisk.org/r/4033/diff/2/?file=67696#file67696line795> > > > > You're currently leaking memory from the regcomp() calls. You need to > > call regfree() on &r_name and &r_value. > > > > Also, should REG_ICASE be one of the flags passed to regcomp? Fixed the leak but the the match should remain case-sensitive. Nothing prevents a user from creating multiple variable names or values differing only in case. If the match was case-insensitive you could only ever hit the first one. - George ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4033/#review13414 ----------------------------------------------------------- On Sept. 29, 2014, 4:11 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4033/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2014, 4:11 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > This patch provides the capability to manipulate templates and categories > with non-unique names via AMI. > > Summary of changes: > > GetConfig and GetConfigJSON: > Added "Filter" parameter: A comma separated list of name_regex=value_regex > expressions which will cause only categories whose variables match all > expressions to be considered. The special variable name TEMPLATES can be > used to control whether templates are included. Passing 'include' as the > value will include templates along with normal categories. Passing 'restrict' > as the value will restrict the operation to ONLY templates. Not specifying a > TEMPLATES expression results in the current default behavior which is to not > include templates. > > Examples: > "GetConfig?Filename=pjsip.conf&Category=myitsp&Filter=type=aor" would return > only 'myitsp' categories with a type of 'aor'. > > "GetConfig?Filename=pjsip.conf&Category=itsp-template&Filter=TEMPLATES=restrict,type=aor" > would return only 'itsp-template' categories that are actually templates > with a type of 'aor'. > > "GetConfig?Filename=pjsip.conf&Filter=type=registration,endpoint=myitsp" > would return only registrations whose corresponding endpoint is 'myitsp'. > > The output from GetConfig and GetConfigJSON also includes variables > indicating if the returned category is a template and the template names a > category inherits from if any. > > UpdateConfig: > NewCat now includes options for allowing duplicate category names, indicating > if the category should be created as a template, and specifying templates the > category should inherit from. The rest of the actions now accept a filter > string as defined above. If there are non-unique category names, you can now > update specific ones based on variable values. > > To facilitate the new capabilities in manager, corresponding changes had to > be made to config, most notably the addition of filter criteria to many of > the APIs. In some cases it was easy to change the references to use the new > prototype but others would have required touching too many files for this > patch so a wrapper with the original prototype was created. Macros couldn't > be used in this case because it would break binary compatibility with modules > such as res_digium_phone that are linked to real symbols. > > > Diffs > ----- > > branches/12/tests/test_sorcery_realtime.c 424097 > branches/12/tests/test_sorcery.c 424097 > branches/12/tests/test_config.c 424097 > branches/12/res/res_sorcery_realtime.c 424097 > branches/12/res/res_sorcery_config.c 424097 > branches/12/pbx/pbx_realtime.c 424097 > branches/12/main/manager.c 424097 > branches/12/main/config.c 424097 > branches/12/include/asterisk/config.h 424097 > branches/12/apps/app_voicemail.c 424097 > branches/12/apps/app_directory.c 424097 > > Diff: https://reviewboard.asterisk.org/r/4033/diff/ > > > Testing > ------- > > Testsuite before and after runs gave the same results. > A set of unit tests for config was added to test_config. They test basic, > filtered and template operations. > > > Thanks, > > George Joseph > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
