[pacman-dev] [PATCH 2/9] conf.c: use masks for siglevel inheritance

Allan McRae allan at archlinux.org
Sat Dec 27 09:12:01 UTC 2014


On 27/04/14 09:56, Andrew Gregory wrote:
> This will allow pacman to parse its config file in a single pass and
> removes the need for the *_SET siglevels in alpm that were only required
> for pacman's siglevel inheritance.
> 
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---
>  lib/libalpm/alpm.h    |  3 --
>  lib/libalpm/be_sync.c |  2 +-
>  src/pacman/conf.c     | 93 ++++++++++++++++++++-------------------------------
>  src/pacman/conf.h     |  5 +++
>  4 files changed, 43 insertions(+), 60 deletions(-)
> 
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index b0adb95..448b8a0 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -115,9 +115,6 @@ typedef enum _alpm_siglevel_t {
>  	ALPM_SIG_DATABASE_MARGINAL_OK = (1 << 12),
>  	ALPM_SIG_DATABASE_UNKNOWN_OK = (1 << 13),
>  
> -	ALPM_SIG_PACKAGE_SET = (1 << 27),
> -	ALPM_SIG_PACKAGE_TRUST_SET = (1 << 28),
> -
>  	ALPM_SIG_USE_DEFAULT = (1 << 31)
>  } alpm_siglevel_t;
>  

OK

> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index df0b1b7..0a131d2 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -698,7 +698,7 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename,
>  	_alpm_log(handle, ALPM_LOG_DEBUG, "registering sync database '%s'\n", treename);
>  
>  #ifndef HAVE_LIBGPGME
> -	if((level &= ~ALPM_SIG_PACKAGE_SET) != 0 && level != ALPM_SIG_USE_DEFAULT) {
> +	if(level != ALPM_SIG_USE_DEFAULT) {
>  		RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL);
>  	}
>  #endif

OK

> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index bcfa12a..ab80373 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -319,12 +319,15 @@ int config_set_arch(const char *arch)
>   * @return 0 on success, 1 on any parsing error
>   */
>  static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage,
> -		const char *file, int linenum)
> +		alpm_siglevel_t *storage_mask, const char *file, int linenum)
>  {
> -	alpm_siglevel_t level = *storage;
> +	alpm_siglevel_t level = *storage, mask = *storage_mask;
>  	alpm_list_t *i;
>  	int ret = 0;
>  
> +#define SLSET(sl) do { level |= (sl); mask |= (sl); } while(0)
> +#define SLUNSET(sl) do { level &= ~(sl); mask |= (sl); } while(0)
> +
>  	/* Collapse the option names into a single bitmasked value */
>  	for(i = values; i; i = alpm_list_next(i)) {
>  		const char *original = i->data, *value;

OK

> @@ -347,51 +350,40 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage,
>  		/* now parse out and store actual flag if it is valid */
>  		if(strcmp(value, "Never") == 0) {
>  			if(package) {
> -				level &= ~ALPM_SIG_PACKAGE;
> -				level |= ALPM_SIG_PACKAGE_SET;
> +				SLUNSET(ALPM_SIG_PACKAGE);
>  			}
>  			if(database) {
> -				level &= ~ALPM_SIG_DATABASE;
> +				SLUNSET(ALPM_SIG_DATABASE);
>  			}
>  		} else if(strcmp(value, "Optional") == 0) {
>  			if(package) {
> -				level |= ALPM_SIG_PACKAGE;
> -				level |= ALPM_SIG_PACKAGE_OPTIONAL;
> -				level |= ALPM_SIG_PACKAGE_SET;
> +				SLSET(ALPM_SIG_PACKAGE | ALPM_SIG_PACKAGE_OPTIONAL);
>  			}
>  			if(database) {
> -				level |= ALPM_SIG_DATABASE;
> -				level |= ALPM_SIG_DATABASE_OPTIONAL;
> +				SLSET(ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL);
>  			}
>  		} else if(strcmp(value, "Required") == 0) {
>  			if(package) {
> -				level |= ALPM_SIG_PACKAGE;
> -				level &= ~ALPM_SIG_PACKAGE_OPTIONAL;
> -				level |= ALPM_SIG_PACKAGE_SET;
> +				SLSET(ALPM_SIG_PACKAGE);
> +				SLUNSET(ALPM_SIG_PACKAGE_OPTIONAL);
>  			}
>  			if(database) {
> -				level |= ALPM_SIG_DATABASE;
> -				level &= ~ALPM_SIG_DATABASE_OPTIONAL;
> +				SLSET(ALPM_SIG_DATABASE);
> +				SLUNSET(ALPM_SIG_DATABASE_OPTIONAL);
>  			}
>  		} else if(strcmp(value, "TrustedOnly") == 0) {
>  			if(package) {
> -				level &= ~ALPM_SIG_PACKAGE_MARGINAL_OK;
> -				level &= ~ALPM_SIG_PACKAGE_UNKNOWN_OK;
> -				level |= ALPM_SIG_PACKAGE_TRUST_SET;
> +				SLUNSET(ALPM_SIG_PACKAGE_MARGINAL_OK | ALPM_SIG_PACKAGE_UNKNOWN_OK);
>  			}
>  			if(database) {
> -				level &= ~ALPM_SIG_DATABASE_MARGINAL_OK;
> -				level &= ~ALPM_SIG_DATABASE_UNKNOWN_OK;
> +				SLUNSET(ALPM_SIG_DATABASE_MARGINAL_OK | ALPM_SIG_DATABASE_UNKNOWN_OK);
>  			}
>  		} else if(strcmp(value, "TrustAll") == 0) {
>  			if(package) {
> -				level |= ALPM_SIG_PACKAGE_MARGINAL_OK;
> -				level |= ALPM_SIG_PACKAGE_UNKNOWN_OK;
> -				level |= ALPM_SIG_PACKAGE_TRUST_SET;
> +				SLSET(ALPM_SIG_PACKAGE_MARGINAL_OK | ALPM_SIG_PACKAGE_UNKNOWN_OK);
>  			}
>  			if(database) {
> -				level |= ALPM_SIG_DATABASE_MARGINAL_OK;
> -				level |= ALPM_SIG_DATABASE_UNKNOWN_OK;
> +				SLSET(ALPM_SIG_DATABASE_MARGINAL_OK | ALPM_SIG_DATABASE_UNKNOWN_OK);
>  			}
>  		} else {
>  			pm_printf(ALPM_LOG_ERROR,

OK

> @@ -402,6 +394,9 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage,
>  		level &= ~ALPM_SIG_USE_DEFAULT;
>  	}
>  
> +#undef SLSET
> +#undef SLUNSET
> +
>  	/* ensure we have sig checking ability and are actually turning it on */
>  	if(!(alpm_capabilities() & ALPM_CAPABILITY_SIGNATURES) &&
>  			level & (ALPM_SIG_PACKAGE | ALPM_SIG_DATABASE)) {
> @@ -413,32 +408,11 @@ static int process_siglevel(alpm_list_t *values, alpm_siglevel_t *storage,
>  
>  	if(!ret) {
>  		*storage = level;
> +		*storage_mask = mask;
>  	}
>  	return ret;
>  }
>  
> -/**
> - * Merge the package entires of two signature verification levels.
> - * @param base initial siglevel
> - * @param over overridden siglevel, derived value is stored here
> - */
> -static void merge_siglevel(alpm_siglevel_t *base, alpm_siglevel_t *over)
> -{
> -	alpm_siglevel_t level = *over;
> -	if(!(level & ALPM_SIG_USE_DEFAULT)) {
> -		if(!(level & ALPM_SIG_PACKAGE_SET)) {
> -			level |= *base & ALPM_SIG_PACKAGE;
> -			level |= *base & ALPM_SIG_PACKAGE_OPTIONAL;
> -		}
> -		if(!(level & ALPM_SIG_PACKAGE_TRUST_SET)) {
> -			level |= *base & ALPM_SIG_PACKAGE_MARGINAL_OK;
> -			level |= *base & ALPM_SIG_PACKAGE_UNKNOWN_OK;
> -		}
> -	}
> -
> -	*over = level;
> -}
> -
>  static int process_cleanmethods(alpm_list_t *values,
>  		const char *file, int linenum)
>  {
> @@ -585,7 +559,8 @@ static int _parse_options(const char *key, char *value,
>  		} else if(strcmp(key, "SigLevel") == 0) {
>  			alpm_list_t *values = NULL;
>  			setrepeatingoption(value, "SigLevel", &values);
> -			if(process_siglevel(values, &config->siglevel, file, linenum)) {
> +			if(process_siglevel(values, &config->siglevel,
> +						&config->siglevel_mask, file, linenum)) {

OK

>  				FREELIST(values);
>  				return 1;
>  			}
> @@ -593,7 +568,8 @@ static int _parse_options(const char *key, char *value,
>  		} else if(strcmp(key, "LocalFileSigLevel") == 0) {
>  			alpm_list_t *values = NULL;
>  			setrepeatingoption(value, "LocalFileSigLevel", &values);
> -			if(process_siglevel(values, &config->localfilesiglevel, file, linenum)) {
> +			if(process_siglevel(values, &config->localfilesiglevel,
> +						&config->localfilesiglevel_mask, file, linenum)) {
>  				FREELIST(values);
>  				return 1;
>  			}
> @@ -601,7 +577,8 @@ static int _parse_options(const char *key, char *value,
>  		} else if(strcmp(key, "RemoteFileSigLevel") == 0) {
>  			alpm_list_t *values = NULL;
>  			setrepeatingoption(value, "RemoteFileSigLevel", &values);
> -			if(process_siglevel(values, &config->remotefilesiglevel, file, linenum)) {
> +			if(process_siglevel(values, &config->remotefilesiglevel,
> +						&config->remotefilesiglevel_mask, file, linenum)) {
>  				FREELIST(values);
>  				return 1;
>  			}

OK

> @@ -727,8 +704,11 @@ static int setup_libalpm(void)
>  
>  	alpm_option_set_default_siglevel(handle, config->siglevel);
>  
> -	merge_siglevel(&config->siglevel, &config->localfilesiglevel);
> -	merge_siglevel(&config->siglevel, &config->remotefilesiglevel);
> +#define SLMERGE(l, m) if(m) { l = (l & (m)) | (config->siglevel & ~(m)); }
> +	SLMERGE(config->localfilesiglevel, config->localfilesiglevel_mask);
> +	SLMERGE(config->remotefilesiglevel, config->remotefilesiglevel_mask);
> +#undef SLMERGE
> +
>  	alpm_option_set_local_file_siglevel(handle, config->localfilesiglevel);
>  	alpm_option_set_remote_file_siglevel(handle, config->remotefilesiglevel);
>  
> @@ -822,10 +802,8 @@ static int _parse_repo(const char *key, char *value, const char *file,
>  			alpm_list_t *values = NULL;
>  			setrepeatingoption(value, "SigLevel", &values);
>  			if(values) {
> -				if(repo->siglevel == ALPM_SIG_USE_DEFAULT) {
> -					repo->siglevel = config->siglevel;
> -				}
> -				ret = process_siglevel(values, &repo->siglevel, file, line);
> +				ret = process_siglevel(values, &repo->siglevel,
> +						&repo->siglevel_mask, file, line);
>  				FREELIST(values);

OK

>  			}
>  		}
> @@ -871,6 +849,9 @@ static int finish_section(struct section_t *section)
>  	}
>  
>  	/* if we are not looking at options sections only, register a db */
> +#define SLMERGE(l, m) if(m) { l = (l & (m)) | (config->siglevel & ~(m)); }
> +	SLMERGE(repo->siglevel, repo->siglevel_mask);
> +#undef SLMERGE

Should this be a function rather than using the same define twice?

>  	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"),
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index 97f006f..42484fb 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -39,6 +39,7 @@ typedef struct __config_repo_t {
>  	alpm_list_t *servers;
>  	alpm_db_usage_t usage;
>  	alpm_siglevel_t siglevel;
> +	alpm_siglevel_t siglevel_mask;
>  } config_repo_t;
>  
>  typedef struct __config_t {
> @@ -95,6 +96,10 @@ typedef struct __config_t {
>  	alpm_siglevel_t localfilesiglevel;
>  	alpm_siglevel_t remotefilesiglevel;
>  
> +	alpm_siglevel_t siglevel_mask;
> +	alpm_siglevel_t localfilesiglevel_mask;
> +	alpm_siglevel_t remotefilesiglevel_mask;
> +
>  	/* conf file options */
>  	/* I Love Candy! */
>  	unsigned short chomp;
> 

OK


More information about the pacman-dev mailing list