[pacman-dev] [PATCH 4/4] Allow VerifySig to act as a default verification in [options]

[Dave Reisner]

On Mon, Mar 28, 2011 at 09:34:57PM +0200, Rémy Oudompheng wrote:
> On Mon 28 March 2011 at 15:15 -0400, Dave Reisner wrote:
> > +/* GPG signature verification option */
> > +typedef enum _pgp_verify_t {
> > +	PM_PGP_VERIFY_UNKNOWN,
> > +	PM_PGP_VERIFY_ALWAYS,
> > +	PM_PGP_VERIFY_OPTIONAL,
> > +	PM_PGP_VERIFY_NEVER
> > +} pgp_verify_t;
> > +
> > +int alpm_db_set_pgp_verify(pmdb_t *db, pgp_verify_t verify);
> 
> Why is this enum name not prefixed by "pm" ?
> 
> Why isn't alpm_db_set_pgp_verify with the other database functions
> (e.g. alpm_db_setserver()) ?
> 

Dan's naming, not mine. Adding the pm prefix makes sense here. I added
it with the signature related functions since it's related to
signatures and not strictly databases (think of pacman -U).

> > --- a/lib/libalpm/signing.c
> > +++ b/lib/libalpm/signing.c
> > @@ -249,6 +249,24 @@ int _alpm_load_signature(const char *sigfile, pmpgpsig_t *pgpsig) {
> >  }
> >  
> >  /**
> > + * Determines the necessity of checking for a valid pgp signature
> > + * @param db the sync db to query
> > + *
> > + * @return signature verification level
> > + */
> > +pgp_verify_t _alpm_get_sigverify_level(pmdb_t *db)
> > +{
> > +	ALPM_LOG_FUNC;
> > +	ASSERT(db != NULL, return PM_PGP_VERIFY_UNKNOWN);
> > +
> > +	if(db->pgp_verify != PM_PGP_VERIFY_UNKNOWN) {
> > +		return db->pgp_verify;
> > +	} else {
> > +		return alpm_option_get_default_sigverify();
> > +	}
> > +}
> > +
> > +/**
> >   * Check the PGP package signature for the given package file.
> >   * @param pkg the package to check
> >   * @return a int value : 0 (valid), 1 (invalid), -1 (an error occured)
> > @@ -270,11 +288,10 @@ int SYMEXPORT alpm_pkg_check_pgp_signature(pmpkg_t *pkg)
> >  int SYMEXPORT alpm_db_check_pgp_signature(pmdb_t *db)
> >  {
> >  	ALPM_LOG_FUNC;
> > -	ASSERT(db != NULL, return(0));
> > +	ASSERT(db != NULL, return 0);
> >  
> >  	return _alpm_gpgme_checksig(_alpm_db_path(db),
> >  			_alpm_db_pgpsig(db));
> >  }
> >  
> > -
> >  /* vim: set ts=2 sw=2 noet: */
> 
> I suggest using ASSERT(db != NULL, RET_ERR(PM_ERR_DB_NULL, ...)); to
> inform users of the error. And you should return -1 if db is NULL
> (in alpm_db_check_signature).
> 
> Regards,
> -- 
> Rémy.
>