From e21b038a231e2743e30915af9575b8c43e9fda1f Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= <amatej@redhat.com>

Date: Tue, 15 Dec 2020 14:15:39 +0100

Subject: [PATCH] Never leave behind .repodata lock on exit (RhBug:1906831

createrepo_c was removing `.repodata` if `exit_val` was se to failure.

Problem is `exit_val` is set only on a couple of places so most of the

failures (no matter how rare) leave `.repodata` behind.

However because on a successful run `.repodata` is renamed to normal

output `repodata` we know that if `.repodata` are present there was an

error and we can delete them.

---

 src/createrepo_c.c      |  5 +----

 src/createrepo_shared.c | 29 +++++++++--------------------

 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/src/createrepo_c.c b/src/createrepo_c.c

index 8c90ebd..eeb871c 100644

--- a/src/createrepo_c.c

+++ b/src/createrepo_c.c

@@ -665,9 +665,6 @@ main(int argc, char **argv)

         exit(EXIT_FAILURE);

     }

-    // Set exit_value pointer used in cleanup handlers

-    cr_set_global_exit_value(&exit_val);

-

     // Setup cleanup handlers

     if (!cr_set_cleanup_handler(lock_dir, tmp_out_repo, &tmp_err)) {

         g_printerr("%s\n", tmp_err->message);

@@ -885,7 +882,7 @@ main(int argc, char **argv)

         char *moduleindex_str = modulemd_module_index_dump_to_string (moduleindex, &tmp_err);

         g_clear_pointer(&moduleindex, g_object_unref);

         if (tmp_err) {

-            g_critical("%s: Cannot cannot dump module index: %s", __func__, tmp_err->message);

+            g_critical("%s: Cannot dump module index: %s", __func__, tmp_err->message);

             free(moduleindex_str);

             g_clear_error(&tmp_err);

             exit(EXIT_FAILURE);

diff --git a/src/createrepo_shared.c b/src/createrepo_shared.c

index f8ef998..d1a37bd 100644

--- a/src/createrepo_shared.c

+++ b/src/createrepo_shared.c

@@ -28,8 +28,6 @@

 #include "misc.h"

 #include "cleanup.h"

-int *global_exit_status = NULL;  // pointer to exit_value used in failure_exit_cleanup

-

 char *global_lock_dir     = NULL;  // Path to .repodata/ dir that is used as a lock

 char *global_tmp_out_repo = NULL;  // Path to temporary repodata directory,

                                    // if NULL that it's same as

@@ -43,18 +41,16 @@ char *global_tmp_out_repo = NULL;  // Path to temporary repodata directory,

  *

  */

 static void

-failure_exit_cleanup()

+exit_cleanup()

 {

-    if (global_exit_status && *global_exit_status != EXIT_SUCCESS) {

-        if (global_lock_dir) {

-            g_debug("Removing %s", global_lock_dir);

-            cr_remove_dir(global_lock_dir, NULL);

-        }

+    if (global_lock_dir) {

+        g_debug("Removing %s", global_lock_dir);

+        cr_remove_dir(global_lock_dir, NULL);

+    }

-        if (global_tmp_out_repo) {

-            g_debug("Removing %s", global_tmp_out_repo);

-            cr_remove_dir(global_tmp_out_repo, NULL);

-        }

+    if (global_tmp_out_repo) {

+        g_debug("Removing %s", global_tmp_out_repo);

+        cr_remove_dir(global_tmp_out_repo, NULL);

     }

 }

@@ -65,16 +61,9 @@ static void

 sigint_catcher(int sig)

 {

     g_message("%s caught: Terminating...", strsignal(sig));

-    *global_exit_status = 1;

     exit(1);

 }

-void

-cr_set_global_exit_value(int *exit_val)

-{

-    global_exit_status = exit_val;

-}

-

 gboolean

 cr_set_cleanup_handler(const char *lock_dir,

                        const char *tmp_out_repo,

@@ -90,7 +79,7 @@ cr_set_cleanup_handler(const char *lock_dir,

         global_tmp_out_repo = NULL;

     // Register on exit cleanup function

-    if (atexit(failure_exit_cleanup))

+    if (atexit(exit_cleanup))

         g_warning("Cannot set exit cleanup function by atexit()");

     // Prepare signal handler configuration