On 03/11/14 at 07:29pm, Sören Brinkmann wrote:
Refactor the upgrade routine to use an array that can be allocated in one step instead of an alpm_list that is gradually extended in loops.
Signed-off-by: Sören Brinkmann <soeren.brinkmann@gmail.com> ---
Looks good; I just have a few small style nitpicks.
src/pacman/upgrade.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c index 11da4dcb5e53..e0cdf564971e 100644 --- a/src/pacman/upgrade.c +++ b/src/pacman/upgrade.c @@ -39,23 +39,25 @@ */ int pacman_upgrade(alpm_list_t *targets) { - int retval = 0; - alpm_list_t *i, *j, *remote = NULL; + int retval = 0, *file_is_remote; + alpm_list_t *i; + unsigned int n, num_targets;
if(targets == NULL) { pm_printf(ALPM_LOG_ERROR, _("no targets specified (use -h for help)\n")); return 1; }
- /* Check for URL targets and process them - */ - for(i = targets; i; i = alpm_list_next(i)) { - int *r = malloc(sizeof(int)); - if(r == NULL) { - pm_printf(ALPM_LOG_ERROR, _("memory exhausted\n")); - return 1; - } + num_targets = alpm_list_count(targets);
+ /* Check for URL targets and process them */ + file_is_remote = malloc(num_targets * sizeof(*file_is_remote));
Our style guidelines require that sizeof() be used with types, not values.
+ if(file_is_remote == NULL) { + pm_printf(ALPM_LOG_ERROR, _("memory exhausted\n")); + return 1; + } + + for(i = targets, n = 0; n < num_targets; i = alpm_list_next(i), n++) {
I would prefer to use i as the conditional check here instead of n since it is such a common idiom throughout our codebase.
if(strstr(i->data, "://")) { char *str = alpm_fetch_pkgurl(config->handle, i->data); if(str == NULL) { @@ -65,13 +67,11 @@ int pacman_upgrade(alpm_list_t *targets) } else { free(i->data); i->data = str; - *r = 1; + file_is_remote[n] = 1; } } else { - *r = 0; + file_is_remote[n] = 0; } - - remote = alpm_list_add(remote, r); }
if(retval) { @@ -85,12 +85,12 @@ int pacman_upgrade(alpm_list_t *targets)
printf(_("loading packages...\n")); /* add targets to the created transaction */ - for(i = targets, j = remote; i; i = alpm_list_next(i), j = alpm_list_next(j)) { + for(i = targets, n = 0; n < num_targets; i = alpm_list_next(i), n++) {
Same as above.
const char *targ = i->data; alpm_pkg_t *pkg; alpm_siglevel_t level;
- if(*(int *)j->data) { + if(file_is_remote[n]) { level = alpm_option_get_remote_file_siglevel(config->handle); } else { level = alpm_option_get_local_file_siglevel(config->handle); @@ -112,7 +112,7 @@ int pacman_upgrade(alpm_list_t *targets) config->explicit_adds = alpm_list_add(config->explicit_adds, pkg); }
- FREELIST(remote); + free(file_is_remote);
if(retval) { trans_release(); -- 1.9.0