On Monday 16 November 2009, Frans Pop wrote: > I think the attached incremental patch addresses all your comments.
Eh, wrong commit. Correct patch attached.
commit 888015f06c122e1fa723f14bb69a527b2ef7585b Author: Frans Pop <f...@debian.org> Date: Sun Nov 15 23:51:38 2009 +0100 More improvements from Jeremy diff --git a/packages/choose-mirror/choose-mirror.c b/packages/choose-mirror/choose-mirror.c index 45c4cf5..43e7344 100644 --- a/packages/choose-mirror/choose-mirror.c +++ b/packages/choose-mirror/choose-mirror.c @@ -159,22 +159,29 @@ void log_invalid_release(const char *name, const char *field) { static int get_release(struct release_t *release, const char *name); /* - * Try to fetch a Release file using its codename and, if successful, check + * Try to fetch a Release file using its codename; if successful, check * that it matches the Release file that was fetched using the suite. + * Returns false only if an invalid Release file was found. */ -static int validate_codename(int *status, const char *codename, const char *suite) { - struct release_t release; - int ret = 0; +static int validate_codename(struct release_t *s_release) { + struct release_t cn_release; + int ret = 1; - if (get_release(&release, codename)) { - *status = release.status; - if (strcmp(release.suite, suite) != 0) - *status &= ~IS_VALID; - ret = 1; + memset(&cn_release, 0, sizeof(struct release_t)); + + /* s_release->name is the codename to check */ + if (get_release(&cn_release, s_release->name)) { + if ((cn_release.status & IS_VALID) && + strcmp(cn_release.suite, s_release->suite) == 0) { + s_release->status |= (cn_release.status & GET_CODENAME); + } else { + s_release->status &= ~IS_VALID; + ret = 0; + } } - free(release.name); - free(release.suite); + free(cn_release.name); + free(cn_release.suite); return ret; } @@ -206,9 +213,6 @@ static int get_release(struct release_t *release, const char *name) { free(hostname); free(directory); - release->name = NULL; - release->suite = NULL; - release->status = 0; if (f != NULL) { while (fgets(line, sizeof(line), f) != NULL) { char *value; @@ -235,17 +239,9 @@ static int get_release(struct release_t *release, const char *name) { /* Check if release can also be gotten using codename */ if ((release->status & IS_VALID) && release->name != NULL && - !(release->status & GET_CODENAME)) { - int cn_status = 0; - if (validate_codename(&cn_status, release->name, name)) { - if (cn_status & IS_VALID) { - release->status |= (cn_status & GET_CODENAME); - } else { - release->status &= ~IS_VALID; - log_invalid_release(name, "Codename"); - } - } - } + !(release->status & GET_CODENAME)) + if (! validate_codename(release)) + log_invalid_release(name, "Codename"); /* In case there is no Codename field */ if ((release->status & IS_VALID) && release->name == NULL) @@ -257,7 +253,13 @@ static int get_release(struct release_t *release, const char *name) { pclose(f); - return (release->name != NULL); + if (release->name != NULL) { + return 1; + } else { + if (release->suite) + free(release->suite); + return 0; + } } static int find_releases(void) { @@ -277,9 +279,13 @@ static int find_releases(void) { DEBCONF_BASE "checking_download"); } + /* Initialize releases; also ensures NULL termination for .name */ + memset(&releases, 0, MAXRELEASES * sizeof(struct release_t)); + /* Get releases for all suites */ if (! base_on_cd) { for (i=0; i < nbr_suites && r < MAXRELEASES; i++) { + memset(&release, 0, sizeof(struct release_t)); if (get_release(&release, suites[i])) { if (release.status & IS_VALID) { if (strcmp(release.name, default_suite) == 0 || @@ -303,6 +309,7 @@ static int find_releases(void) { /* Try to get release using the default "suite" */ if (! bad_mirror && (base_on_cd || r == 0)) { + memset(&release, 0, sizeof(struct release_t)); if (get_release(&release, default_suite)) { if (release.status & IS_VALID) { release.status |= IS_DEFAULT; @@ -313,9 +320,6 @@ static int find_releases(void) { } } - /* Mark end of list */ - releases[r].name = NULL; - if (show_progress) { debconf_progress_step(debconf, nbr_suites); debconf_progress_stop(debconf); @@ -324,6 +328,11 @@ static int find_releases(void) { free(default_suite); if (r == 0 || bad_mirror) { + if (release.name) + free(release.name); + if (release.suite) + free(release.suite); + debconf_input(debconf, "critical", DEBCONF_BASE "bad"); if (debconf_go(debconf) == 30) exit(10); /* back up to menu */ @@ -652,16 +661,18 @@ static int check_mirror(void) { } static int choose_suite(void) { - char **choices_c, **choices, *list; + char *choices_c[MAXRELEASES], *choices[MAXRELEASES], *list; int i, ret; ret = find_releases(); if (ret) return ret; + /* Also ensures NULL termination */ + memset(choices, 0, MAXRELEASES * sizeof(char *)); + memset(choices_c, 0, MAXRELEASES * sizeof(char *)); + /* Arrays can never overflow as we've already checked releases */ - choices_c = malloc(MAXRELEASES * sizeof(char*)); - choices = malloc(MAXRELEASES * sizeof(char*)); for (i=0; releases[i].name != NULL; i++) { char *name; @@ -681,16 +692,18 @@ static int choose_suite(void) { free(name); } - choices_c[i] = NULL; list = debconf_list(choices_c); debconf_subst(debconf, DEBCONF_BASE "suite", "CHOICES-C", list); - choices[i] = NULL; + free(list); list = debconf_list(choices); debconf_subst(debconf, DEBCONF_BASE "suite", "CHOICES", list); - free(choices_c); - free(choices); free(list); + for (i=0; choices[i] != NULL; i++) { + free(choices_c[i]); + free(choices[i]); + } + /* If the base system can be installed from CD, don't allow to * select a different suite */ @@ -722,7 +735,6 @@ int set_codename (void) { codename = releases[i].suite; debconf_set(debconf, DEBCONF_BASE "codename", codename); di_log(DI_LOG_LEVEL_INFO, "codename set to: %s", codename); - free(codename); break; } } @@ -787,6 +799,7 @@ int check_arch (void) { } int main (int argc, char **argv) { + int i; /* Use a state machine with a function to run in each state */ int state = 0; int (*states[])() = { @@ -827,5 +840,11 @@ int main (int argc, char **argv) { else state++; } + + for (i=0; releases[i].name != NULL; i++) { + free(releases[i].name); + free(releases[i].suite); + } + return (state >= 0) ? 0 : 10; /* backed all the way out */ }