[asterisk-bugs] [JIRA] (ASTERISK-23383) Wrong sense test on stat return code causes unchanged config check to break with include files.

[Richard Mudgett (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-23383?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Richard Mudgett updated ASTERISK-23383:
---------------------------------------

    Description: 
Whilst trying to understand why an old version (1.6.1.0) was not treating configuration files as unchanged during a reload if there were include files, we discovered the equivalent of the following code, which clearly has the wrong sense test on the result of stat.  This quote has been taken from the trunk version, but it has remained unchanged since the 1.8 branch version, and differs only in the use of direct assignments before that.

In config_cache_attribute in main/config.c

{code}
if (!stat(configfile, &statbuf)) {
       cfmstat_clear(cfmtime);
} else {
       cfmstat_save(cfmtime, &statbuf);
}
{code}

Whilst I still haven't worked out why the including files cfmtime needs updating here at all, the test is clearly wrong, as it only uses the stat result when it is invalid!

The main consequence of this, other than wasted processing, as the result of doing full reloads on unchanged files, is that queue statistics get reset on a null reload.  However we are also investigating a possible race in the queue statistics update, which may or may not still be current.

  was:
Whilst trying to understand why an old version (1.6.1.0) was not treating configuration files as unchanged during a reload if there were include files, we discovered the equivalent of the following code, which clearly has the wrong sense test on the result of stat.  This quote has been taken from the trunk version, but it has remained unchanged since the 1.8 branch version, and differs only in the use of direct assignments before that.

In config_cache_attribute in main/config.c

[quote]
       if (!stat(configfile, &statbuf)) {
               cfmstat_clear(cfmtime);
       } else {
               cfmstat_save(cfmtime, &statbuf);
       }
[quote]

Whilst I still haven't worked out why the including files cfmtime needs updating here at all, the test is clearly wrong, as it only uses the stat result when it is invalid!

The main consequence of this, other than wasted processing, as the result of doing full reloads on unchanged files, is that queue statistics get reset on a null reload.  However we are also investigating a possible race in the queue statistics update, which may or may not still be current.

    
> Wrong sense test on stat return code causes unchanged config check to break with include files.
> -----------------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-23383
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-23383
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Core/Configuration
>    Affects Versions: SVN, 1.8.25.0
>         Environment: CentOS, although not really environment specific.
>            Reporter: David Woolley
>            Severity: Minor
>
> Whilst trying to understand why an old version (1.6.1.0) was not treating configuration files as unchanged during a reload if there were include files, we discovered the equivalent of the following code, which clearly has the wrong sense test on the result of stat.  This quote has been taken from the trunk version, but it has remained unchanged since the 1.8 branch version, and differs only in the use of direct assignments before that.
> In config_cache_attribute in main/config.c
> {code}
> if (!stat(configfile, &statbuf)) {
>        cfmstat_clear(cfmtime);
> } else {
>        cfmstat_save(cfmtime, &statbuf);
> }
> {code}
> Whilst I still haven't worked out why the including files cfmtime needs updating here at all, the test is clearly wrong, as it only uses the stat result when it is invalid!
> The main consequence of this, other than wasted processing, as the result of doing full reloads on unchanged files, is that queue statistics get reset on a null reload.  However we are also investigating a possible race in the queue statistics update, which may or may not still be current.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.asterisk.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira