[pacman-dev] [PATCH 1/9] conf.c: store repo settings in dedicated struct
Allan McRae
allan at archlinux.org
Sat Dec 27 08:26:27 UTC 2014
On 27/04/14 09:56, Andrew Gregory wrote:
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---
> src/pacman/conf.c | 88 +++++++++++++++++++++++++++++--------------------------
> src/pacman/conf.h | 9 ++++++
> 2 files changed, 55 insertions(+), 42 deletions(-)
>
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index f75f3a7..bcfa12a 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -151,6 +151,16 @@ int config_free(config_t *oldconfig)
> return 0;
> }
>
> +void config_repo_free(config_repo_t *repo)
> +{
> + if(repo == NULL) {
> + return;
> + }
> + free(repo->name);
> + FREELIST(repo->servers);
> + free(repo);
> +}
> +
OK
> /** Helper function for download_with_xfercommand() */
> static char *get_filename(const char *url)
> {
> @@ -750,14 +760,10 @@ static int setup_libalpm(void)
> * calling library methods.
> */
> struct section_t {
> - /* useful for all sections */
> const char *name;
> + config_repo_t *repo;
> int is_options;
> int parse_options;
> - /* db section option gathering */
> - alpm_list_t *servers;
> - alpm_siglevel_t siglevel;
> - alpm_db_usage_t usage;
> };
>
OK
> static int process_usage(alpm_list_t *values, alpm_db_usage_t *usage,
> @@ -798,6 +804,7 @@ static int _parse_repo(const char *key, char *value, const char *file,
> int line, struct section_t *section)
> {
> int ret = 0;
> + config_repo_t *repo = section->repo;
>
OK
> if(strcmp(key, "Server") == 0) {
> if(!value) {
> @@ -805,7 +812,7 @@ static int _parse_repo(const char *key, char *value, const char *file,
> file, line, key);
> ret = 1;
> } else {
> - section->servers = alpm_list_add(section->servers, strdup(value));
> + repo->servers = alpm_list_add(repo->servers, strdup(value));
OK
> }
> } else if(strcmp(key, "SigLevel") == 0) {
> if(!value) {
> @@ -815,10 +822,10 @@ static int _parse_repo(const char *key, char *value, const char *file,
> alpm_list_t *values = NULL;
> setrepeatingoption(value, "SigLevel", &values);
> if(values) {
> - if(section->siglevel == ALPM_SIG_USE_DEFAULT) {
> - section->siglevel = config->siglevel;
> + if(repo->siglevel == ALPM_SIG_USE_DEFAULT) {
> + repo->siglevel = config->siglevel;
> }
> - ret = process_siglevel(values, §ion->siglevel, file, line);
> + ret = process_siglevel(values, &repo->siglevel, file, line);
> FREELIST(values);
> }
> }
OK
> @@ -826,7 +833,7 @@ static int _parse_repo(const char *key, char *value, const char *file,
> alpm_list_t *values = NULL;
> setrepeatingoption(value, "Usage", &values);
> if(values) {
> - if(process_usage(values, §ion->usage, file, line)) {
> + if(process_usage(values, &repo->usage, file, line)) {
> FREELIST(values);
> return 1;
> }
OK
> @@ -835,7 +842,7 @@ static int _parse_repo(const char *key, char *value, const char *file,
> } else {
> pm_printf(ALPM_LOG_WARNING,
> _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"),
> - file, line, key, section->name);
> + file, line, key, repo->name);
> }
OK
>
> return ret;
> @@ -852,51 +859,42 @@ static int _parse_repo(const char *key, char *value, const char *file,
> */
> static int finish_section(struct section_t *section)
> {
> - int ret = 0;
> alpm_list_t *i;
> alpm_db_t *db;
> + config_repo_t *repo = section->repo;
>
> pm_printf(ALPM_LOG_DEBUG, "config: finish section '%s'\n", section->name);
>
> /* parsing options (or nothing)- nothing to do except free the pieces */
> - if(!section->name || section->parse_options || section->is_options) {
> - goto cleanup;
> + if(section->parse_options || !section->repo) {
> + return 0;
> }
>
> /* if we are not looking at options sections only, register a db */
> - db = alpm_register_syncdb(config->handle, section->name, section->siglevel);
> + db = alpm_register_syncdb(config->handle, repo->name, repo->siglevel);
> if(db == NULL) {
> pm_printf(ALPM_LOG_ERROR, _("could not register '%s' database (%s)\n"),
> - section->name, alpm_strerror(alpm_errno(config->handle)));
> - ret = 1;
> - goto cleanup;
> + repo->name, alpm_strerror(alpm_errno(config->handle)));
> + return 1;
> }
>
> pm_printf(ALPM_LOG_DEBUG,
> - "setting usage of %d for %s repoistory\n",
> - section->usage == 0 ? ALPM_DB_USAGE_ALL : section->usage,
> - section->name);
> - alpm_db_set_usage(db, section->usage == 0 ? ALPM_DB_USAGE_ALL : section->usage);
> + "setting usage of %d for %s repository\n",
> + repo->usage == 0 ? ALPM_DB_USAGE_ALL : repo->usage,
> + repo->name);
> + alpm_db_set_usage(db, repo->usage == 0 ? ALPM_DB_USAGE_ALL : repo->usage);
>
> - for(i = section->servers; i; i = alpm_list_next(i)) {
> + for(i = repo->servers; i; i = alpm_list_next(i)) {
> char *value = i->data;
> if(_add_mirror(db, value) != 0) {
> pm_printf(ALPM_LOG_ERROR,
> _("could not add mirror '%s' to database '%s' (%s)\n"),
> - value, section->name, alpm_strerror(alpm_errno(config->handle)));
> - ret = 1;
> - goto cleanup;
> + value, repo->name, alpm_strerror(alpm_errno(config->handle)));
> + return 1;
> }
> - free(value);
> }
>
> -cleanup:
> - alpm_list_free(section->servers);
> - section->servers = NULL;
> - section->siglevel = ALPM_SIG_USE_DEFAULT;
> - section->name = NULL;
> - section->usage = 0;
> - return ret;
> + return 0;
> }
>
> static int _parse_directive(const char *file, int linenum, const char *name,
> @@ -904,13 +902,21 @@ static int _parse_directive(const char *file, int linenum, const char *name,
> {
> struct section_t *section = data;
> if(!key && !value) {
> - int ret = finish_section(section);
> - pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name);
> + int ret = finish_section(data);
Why change "section" to "data" here?
> section->name = name;
> - if(name && strcmp(name, "options") == 0) {
> + pm_printf(ALPM_LOG_DEBUG, "config: new section '%s'\n", name);
> + config_repo_free(section->repo);
> + section->repo = NULL;
> + section->is_options = 0;
> + if(!name) {
> + /* end of file, do nothing */
> + } else if(strcmp(name, "options") == 0) {
> section->is_options = 1;
> - } else {
> - section->is_options = 0;
> + } else if(!section->parse_options) {
> + section->repo = calloc(sizeof(config_repo_t), 1);
> + section->repo->name = strdup(name);
So section->name and section->repo->name carry two copies of the section
name?
> + section->repo->siglevel = ALPM_SIG_USE_DEFAULT;
> + section->repo->usage = 0;
> }
> return ret;
> }
> @@ -924,7 +930,7 @@ static int _parse_directive(const char *file, int linenum, const char *name,
> if(section->parse_options && section->is_options) {
> /* we are either in options ... */
> return _parse_options(key, value, file, linenum);
> - } else if(!section->parse_options && !section->is_options) {
> + } else if(!section->parse_options && section->repo) {
OK
> /* ... or in a repo section */
> return _parse_repo(key, value, file, linenum, section);
> }
> @@ -941,8 +947,6 @@ int parseconfig(const char *file)
> int ret;
> struct section_t section;
> memset(§ion, 0, sizeof(struct section_t));
> - section.siglevel = ALPM_SIG_USE_DEFAULT;
> - section.usage = 0;
OK
> /* the config parse is a two-pass affair. We first parse the entire thing for
> * the [options] section so we can get all default and path options set.
> * Next, we go back and parse everything but [options]. */
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index e8cac50..97f006f 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -34,6 +34,13 @@ typedef struct __colstr_t {
> const char *nocolor;
> } colstr_t;
>
> +typedef struct __config_repo_t {
> + char *name;
> + alpm_list_t *servers;
> + alpm_db_usage_t usage;
> + alpm_siglevel_t siglevel;
> +} config_repo_t;
> +
OK
> typedef struct __config_t {
> unsigned short op;
> unsigned short quiet;
> @@ -205,6 +212,8 @@ void enable_colors(int colors);
> config_t *config_new(void);
> int config_free(config_t *oldconfig);
>
> +void config_repo_free(config_repo_t *repo);
> +
> int config_set_arch(const char *arch);
> int parseconfig(const char *file);
> #endif /* _PM_CONF_H */
>
More information about the pacman-dev
mailing list