chiark / gitweb /
job: allow job_free() only on already unlinked jobs
authorMichal Schmidt <mschmidt@redhat.com>
Thu, 19 Apr 2012 21:20:34 +0000 (23:20 +0200)
committerMichal Schmidt <mschmidt@redhat.com>
Fri, 20 Apr 2012 15:12:27 +0000 (17:12 +0200)
job_free() is IMO too helpful when it unlinks the job from the transaction.
The callers should ensure the job is already unlinked before freeing.
The added assertions check if anyone gets it wrong.

src/core/job.c
src/core/manager.c
src/core/manager.h

index f3c76d6..5ea717e 100644 (file)
@@ -71,8 +71,10 @@ void job_free(Job *j) {
                 j->installed = false;
         }
 
                 j->installed = false;
         }
 
-        /* Detach from next 'smaller' objects */
-        manager_transaction_unlink_job(j->manager, j, true);
+        assert(!j->transaction_prev);
+        assert(!j->transaction_next);
+        assert(!j->subject_list);
+        assert(!j->object_list);
 
         if (j->in_run_queue)
                 LIST_REMOVE(Job, run_queue, j->manager->run_queue, j);
 
         if (j->in_run_queue)
                 LIST_REMOVE(Job, run_queue, j->manager->run_queue, j);
index 9505629..aa918f1 100644 (file)
@@ -662,13 +662,15 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         return r;
 }
 
         return r;
 }
 
+static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies);
+
 static void transaction_delete_job(Manager *m, Job *j, bool delete_dependencies) {
         assert(m);
         assert(j);
 
         /* Deletes one job from the transaction */
 
 static void transaction_delete_job(Manager *m, Job *j, bool delete_dependencies) {
         assert(m);
         assert(j);
 
         /* Deletes one job from the transaction */
 
-        manager_transaction_unlink_job(m, j, delete_dependencies);
+        transaction_unlink_job(m, j, delete_dependencies);
 
         if (!j->installed)
                 job_free(j);
 
         if (!j->installed)
                 job_free(j);
@@ -710,8 +712,10 @@ static void transaction_abort(Manager *m) {
         while ((j = hashmap_first(m->transaction_jobs)))
                 if (j->installed)
                         transaction_delete_job(m, j, true);
         while ((j = hashmap_first(m->transaction_jobs)))
                 if (j->installed)
                         transaction_delete_job(m, j, true);
-                else
+                else {
+                        transaction_unlink_job(m, j, true);
                         job_free(j);
                         job_free(j);
+                }
 
         assert(hashmap_isempty(m->transaction_jobs));
 
 
         assert(hashmap_isempty(m->transaction_jobs));
 
@@ -1441,6 +1445,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o
         LIST_PREPEND(Job, transaction, f, j);
 
         if (hashmap_replace(m->transaction_jobs, unit, f) < 0) {
         LIST_PREPEND(Job, transaction, f, j);
 
         if (hashmap_replace(m->transaction_jobs, unit, f) < 0) {
+                LIST_REMOVE(Job, transaction, f, j);
                 job_free(j);
                 return NULL;
         }
                 job_free(j);
                 return NULL;
         }
@@ -1453,7 +1458,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o
         return j;
 }
 
         return j;
 }
 
-void manager_transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) {
+static void transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies) {
         assert(m);
         assert(j);
 
         assert(m);
         assert(j);
 
index 39e16ae..0e9c0d7 100644 (file)
@@ -257,8 +257,6 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode
 void manager_dump_units(Manager *s, FILE *f, const char *prefix);
 void manager_dump_jobs(Manager *s, FILE *f, const char *prefix);
 
 void manager_dump_units(Manager *s, FILE *f, const char *prefix);
 void manager_dump_jobs(Manager *s, FILE *f, const char *prefix);
 
-void manager_transaction_unlink_job(Manager *m, Job *j, bool delete_dependencies);
-
 void manager_clear_jobs(Manager *m);
 
 unsigned manager_dispatch_load_queue(Manager *m);
 void manager_clear_jobs(Manager *m);
 
 unsigned manager_dispatch_load_queue(Manager *m);