diff --git a/src/core/transaction.c b/src/core/transaction.c index 1e5b88af4d..5fa21e080b 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -325,32 +325,6 @@ static bool job_matters_to_anchor(Job *job) { return false; } -static char* merge_unit_ids(const char* unit_log_field, char * const* pairs) { - _cleanup_free_ char *ans = NULL; - size_t size = 0; - - assert(unit_log_field); - - STRV_FOREACH_PAIR(unit_id, job_type, pairs) { - size_t next; - - if (size > 0) - ans[size - 1] = '\n'; - - next = strlen(unit_log_field) + strlen(*unit_id); - if (!GREEDY_REALLOC(ans, size + next + 1)) - return NULL; - - sprintf(ans + size, "%s%s", unit_log_field, *unit_id); - size += next + 1; - } - - if (!ans) - return strdup(""); - - return TAKE_PTR(ans); -} - static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsigned generation, sd_bus_error *e) { static const UnitDependencyAtom directions[] = { @@ -369,8 +343,8 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi /* Have we seen this before? */ if (j->generation == generation) { - Job *k, *delete = NULL; - _cleanup_free_ char **array = NULL, *unit_ids = NULL; + _cleanup_free_ char **array = NULL; + Job *delete = NULL; /* If the marker is NULL we have been here already and decided the job was loop-free from * here. Hence shortcut things and return right-away. */ @@ -380,7 +354,7 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi /* So, the marker is not NULL and we already have been here. We have a cycle. Let's try to * break it. We go backwards in our path and try to find a suitable job to remove. We use the * marker to find our way back, since smart how we are we stored our way back in there. */ - for (k = from; k; k = ((k->generation == generation && k->marker != k) ? k->marker : NULL)) { + for (Job *k = from; k; k = (k->generation == generation && k->marker != k) ? k->marker : NULL) { /* For logging below */ if (strv_push_pair(&array, k->unit->id, (char*) job_type_to_string(k->type)) < 0) @@ -395,26 +369,26 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi break; } - unit_ids = merge_unit_ids(unit_log_field(j->unit), array); /* ignore error */ - - size_t m = strv_length(array); + _cleanup_free_ char *unit_ids = NULL; + STRV_FOREACH_PAIR(unit_id, job_type, array) + (void) strextendf_with_separator(&unit_ids, "\n", "%s%s", unit_log_field(j->unit), *unit_id); _cleanup_free_ char *cycle_path_text = strdup("Found ordering cycle"); - if (m > 0) { - (void) strextendf(&cycle_path_text, " on %s/%s", array[0], array[1]); - if (m > 2) - (void) strextendf(&cycle_path_text, "; has dependency on %s/%s", array[2], array[3]); - } + if (!strv_isempty(array)) { + (void) strextendf(&cycle_path_text, ": %s/%s", array[0], array[1]); - STRV_FOREACH_PAIR(unit_id, job_type, strv_skip(array, 4)) - (void) strextendf(&cycle_path_text, ", %s/%s", *unit_id, *job_type); + STRV_FOREACH_PAIR(unit_id, job_type, strv_skip(array, 2)) + (void) strextendf(&cycle_path_text, " after %s/%s", *unit_id, *job_type); + + (void) strextendf(&cycle_path_text, " - after %s", array[0]); + } /* logging for j not k here to provide a consistent narrative */ if (cycle_path_text) log_struct(LOG_ERR, LOG_UNIT_MESSAGE(j->unit, "%s", cycle_path_text), LOG_MESSAGE_ID(SD_MESSAGE_UNIT_ORDERING_CYCLE_STR), - LOG_ITEM("%s", strna(unit_ids))); + LOG_ITEM("%s", strempty(unit_ids))); if (delete) { const char *status; @@ -427,7 +401,7 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi LOG_MESSAGE_ID(SD_MESSAGE_DELETING_JOB_BECAUSE_ORDERING_CYCLE_STR), LOG_ITEM("DELETED_UNIT=%s", delete->unit->id), LOG_ITEM("DELETED_TYPE=%s", job_type_to_string(delete->type)), - LOG_ITEM("%s", strna(unit_ids))); + LOG_ITEM("%s", strempty(unit_ids))); if (log_get_show_color()) status = ANSI_HIGHLIGHT_RED " SKIP " ANSI_NORMAL; @@ -447,7 +421,7 @@ static int transaction_verify_order_one(Transaction *tr, Job *j, Job *from, unsi LOG_UNIT_MESSAGE(j->unit, "Unable to break cycle starting with %s/%s", j->unit->id, job_type_to_string(j->type)), LOG_MESSAGE_ID(SD_MESSAGE_CANT_BREAK_ORDERING_CYCLE_STR), - LOG_ITEM("%s", strna(unit_ids))); + LOG_ITEM("%s", strempty(unit_ids))); return sd_bus_error_setf(e, BUS_ERROR_TRANSACTION_ORDER_IS_CYCLIC, "Transaction order is cyclic. See system logs for details.");