From 26f3d71662de4ec0b105bfa1d8ec6635000aa3f6 Mon Sep 17 00:00:00 2001
From: Kirpa Sergey <Kirpa.Sergey@gmail.com>
Date: Wed, 20 Mar 2019 01:20:40 +0200
Subject: [PATCH 1/2] Cleanup backups after saving in MVC

Refactoring: Move legacy function cleanup_backups() to
OPNsense/Core/Config::cleanupBackups()
---
 src/etc/inc/config.inc                        | 28 --------------
 .../mvc/app/library/OPNsense/Core/Config.php  | 37 +++++++++++++++++++
 2 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/src/etc/inc/config.inc b/src/etc/inc/config.inc
index c4abc84b6..1c4e9fc87 100644
--- a/src/etc/inc/config.inc
+++ b/src/etc/inc/config.inc
@@ -164,9 +164,6 @@ function write_config($desc = '', $backup = true)
         configd_run('filter sync load');
     }
 
-    /* cleanup backups */
-    cleanup_backups();
-
     /* on succesfull save, serialize config back to global */
     $config = $cnf->toArray(listtags());
 
@@ -218,31 +215,6 @@ function security_checks_disabled()
     return file_exists('/tmp/disable_security_checks');
 }
 
-/**
- * remove old backups
- */
-function cleanup_backups()
-{
-    global $config;
-
-    if (isset($config['system']['backupcount']) && is_numeric($config['system']['backupcount']) && ($config['system']['backupcount'] >= 0)) {
-        $revisions = intval($config['system']['backupcount']);
-    } else {
-        /* XXX this value used to be left out of the config */
-        $revisions = 60;
-    }
-
-    $cnf = OPNsense\Core\Config::getInstance();
-
-    $cnt=1;
-    foreach ($cnf->getBackups() as $filename) {
-        if ($cnt > $revisions ) {
-            @unlink($filename);
-        }
-        ++$cnt ;
-    }
-}
-
 function &config_read_array()
 {
     global $config;
diff --git a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
index 85bedc22b..d879d1a2d 100644
--- a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
+++ b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
@@ -530,6 +530,40 @@ class Config extends Singleton
         }
     }
 
+    /**
+     * remove old backups
+     */
+    public function cleanupBackups()
+    {
+        /* XXX this value used to be left out of the config */
+        $revisions = 60;
+
+        try {
+            $obj = $this->object();
+
+            if (isset($obj->system->backupcount)) {
+                $backupcount = $obj->system->backupcount;
+
+                if (is_numeric($backupcount)) {
+                    $backupcount = intval($backupcount);
+
+                    if ($backupcount >= 0) {
+                        $revisions = $backupcount;
+                    }
+                }
+            }
+        } catch (\Exception $e) {
+        }
+
+        $cnt = 1;
+        foreach ($this->getBackups() as $filename) {
+            if ($cnt > $revisions ) {
+                @unlink($filename);
+            }
+            ++$cnt ;
+        }
+    }
+
     /**
      * save config to filesystem
      * @param array|null $revision revision tag (associative array)
@@ -562,6 +596,9 @@ class Config extends Singleton
                 throw new ConfigException("Unable to lock config");
             }
         }
+
+        /* cleanup backups */
+        $this->cleanupBackups();
     }
 
     /**

From cfbaafb8d592c9ae0a1b6aeba7326733e679c0d1 Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Wed, 20 Mar 2019 09:26:51 +0100
Subject: [PATCH 2/2] config, remove old backups after save.
 https://github.com/opnsense/core/pull/3352 although the idea is good, the
 code quality really needs attention.

---
 .../mvc/app/library/OPNsense/Core/Config.php  | 25 +++++--------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
index d879d1a2d..91a624fe2 100644
--- a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
+++ b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
@@ -533,26 +533,13 @@ class Config extends Singleton
     /**
      * remove old backups
      */
-    public function cleanupBackups()
+    private function cleanupBackups()
     {
-        /* XXX this value used to be left out of the config */
-        $revisions = 60;
-
-        try {
-            $obj = $this->object();
-
-            if (isset($obj->system->backupcount)) {
-                $backupcount = $obj->system->backupcount;
-
-                if (is_numeric($backupcount)) {
-                    $backupcount = intval($backupcount);
-
-                    if ($backupcount >= 0) {
-                        $revisions = $backupcount;
-                    }
-                }
-            }
-        } catch (\Exception $e) {
+        if ($this->statusIsValid && isset($this->simplexml->system->backupcount)
+                && intval($this->simplexml->system->backupcount) >= 0) {
+            $revisions = intval($this->simplexml->system->backupcount);
+        } else {
+            $revisions = 60;
         }
 
         $cnt = 1;