[pacman-dev] [PATCH] Style: Removing unecessary indentation level.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com> --- src/pacman/sync.c | 84 ++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 7dc69079..9e35053f 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -816,49 +816,51 @@ int sync_prepare_execute(void) goto cleanup; } - if(alpm_trans_commit(config->handle, &data) == -1) { - alpm_errno_t err = alpm_errno(config->handle); - pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction (%s)\n"), - alpm_strerror(err)); - switch(err) { - case ALPM_ERR_FILE_CONFLICTS: - for(i = data; i; i = alpm_list_next(i)) { - alpm_fileconflict_t *conflict = i->data; - switch(conflict->type) { - case ALPM_FILECONFLICT_TARGET: - printf(_("%s exists in both '%s' and '%s'\n"), - conflict->file, conflict->target, conflict->ctarget); - break; - case ALPM_FILECONFLICT_FILESYSTEM: - if(conflict->ctarget[0]) { - printf(_("%s: %s exists in filesystem (owned by %s)\n"), - conflict->target, conflict->file, conflict->ctarget); - } else { - printf(_("%s: %s exists in filesystem\n"), - conflict->target, conflict->file); - } - break; - } - alpm_fileconflict_free(conflict); - } - break; - case ALPM_ERR_PKG_INVALID: - case ALPM_ERR_PKG_INVALID_CHECKSUM: - case ALPM_ERR_PKG_INVALID_SIG: - for(i = data; i; i = alpm_list_next(i)) { - char *filename = i->data; - printf(_("%s is invalid or corrupted\n"), filename); - free(filename); - } - break; - default: - break; - } - /* TODO: stderr? */ - printf(_("Errors occurred, no packages were upgraded.\n")); - retval = 1; + if(alpm_trans_commit(config->handle, &data) != -1) goto cleanup; + + alpm_errno_t err = alpm_errno(config->handle); + pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction (%s)\n"), + alpm_strerror(err)); + switch(err) { + case ALPM_ERR_FILE_CONFLICTS: + for(i = data; i; i = alpm_list_next(i)) { + alpm_fileconflict_t *conflict = i->data; + switch(conflict->type) { + case ALPM_FILECONFLICT_TARGET: + printf(_("%s exists in both '%s' and '%s'\n"), + conflict->file, conflict->target, conflict->ctarget); + break; + case ALPM_FILECONFLICT_FILESYSTEM: + if(conflict->ctarget[0]) { + printf(_("%s: %s exists in filesystem (owned by %s)\n"), + conflict->target, conflict->file, conflict->ctarget); + } else { + printf(_("%s: %s exists in filesystem\n"), + conflict->target, conflict->file); + } + break; + } + alpm_fileconflict_free(conflict); + } + break; + case ALPM_ERR_PKG_INVALID: + case ALPM_ERR_PKG_INVALID_CHECKSUM: + case ALPM_ERR_PKG_INVALID_SIG: + for(i = data; i; i = alpm_list_next(i)) { + char *filename = i->data; + printf(_("%s is invalid or corrupted\n"), filename); + free(filename); + } + break; + default: + break; } + /* TODO: stderr? */ + printf(_("Errors occurred, no packages were upgraded.\n")); + retval = 1; + goto cleanup; + /* Step 4: release transaction resources */ cleanup: -- 2.21.0
On 04/04/19 at 12:21am, Leonardo Bras wrote:
Signed-off-by: Leonardo Bras <leobras.c@gmail.com> --- src/pacman/sync.c | 84 ++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 41 deletions(-)
I appreciate the reduction of indentation, but I think this does more harm than good. It breaks our official style rules because err is no longer declared at the top of its block; it makes sync_prepare_execute inconsistent because the trans_prepare error is still handled in an if block; finally, it makes the error branch look like the primary execution branch instead of an exception. I would move the error printing out of sync_prepare_execute altogether into print_{prepare,commit}_error functions. That will also have the benefit of shortening sync_prepare_execute, which is a bit long.
participants (2)
-
Andrew Gregory
-
Leonardo Bras