----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3575/#review12012 -----------------------------------------------------------
/branches/1.8/main/config.c <https://reviewboard.asterisk.org/r/3575/#comment21949> Just for readability, I'd put a space between the end of the statement and the comment: strcpy(dst, filename); /* Safe */ There's a few other places this applies to as well. /branches/1.8/main/config.c <https://reviewboard.asterisk.org/r/3575/#comment21951> The indentation here is weird. Is it intentional that the following do loop is included with the else of the glob statement? If so, a comment at least indicating that would be nice. /branches/1.8/main/config.c <https://reviewboard.asterisk.org/r/3575/#comment21950> Just to confirm, we no longer need to glob here due to always reloading in the case of a wildcard/directory include, correct? /branches/1.8/main/config.c <https://reviewboard.asterisk.org/r/3575/#comment21952> The globbuf could be free'd earlier than this now (in the first #ifdef block testing for AST_INCLUDE_GLOB), which would remove the need for this #ifdef. - Matt Jordan On May 29, 2014, 5:11 p.m., rmudgett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3575/ > ----------------------------------------------------------- > > (Updated May 29, 2014, 5:11 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23683 > https://issues.asterisk.org/jira/browse/ASTERISK-23683 > > > Repository: Asterisk > > > Description > ------- > > The twisted logic determining if a config file should be reloaded was mostly > broken and disabled. The incorrect test that ASTERISK-23383 fixed actually > reenabled the broken logic. The incorrect test was causing the timestamp to > always be cleared which caused config files with includes to always be > reloaded. > > * Made wildcard includes always cause a reload. Determining if a file was > deleted cannot be determined without restructuring the cache to determine if > any files are missing from the last files actually loaded. Also without > refactoring config_text_file_load(), the glob loop couldn't check more than > one file for changes anyway. > > * Made remove the cache entry if the file no longer exists when trying to get > its timestamp or it is no longer a regular file. This fixes the corner case > where the file was loaded, then deleted, then the config reloaded, then the > file restored with the same timestamp, and then the config reloaded again. > > * Made remove the cache entry include list when actually loading the file. > This gets rid of any stale includes the file had from the last time the file > was loaded. > > > Diffs > ----- > > /branches/1.8/main/config.c 414853 > > Diff: https://reviewboard.asterisk.org/r/3575/diff/ > > > Testing > ------- > > Made sip.conf include a wildcard pattern. The config was always reloaded > even if no files were touched. > > Made sip.conf include several specific files. The config was reloaded when > any file was touched. > > Edited sip.conf to change which files the file included. Verified with debug > statements added to determine that the old include files were no longer > checked on subsequent reload attempts. > > > Thanks, > > rmudgett > >
-- _____________________________________________________________________ -- 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
