On 5/1/21 10:36 am, Emil Velikov via pacman-dev wrote:
> Currently default_pkg_ops is accessed in two different ways.
>
> There is get_file_pkg_ops (in be_package.c) creating a local once-off
> 'tweaked' copy. As well as load_pkg_for_entry (be_sync.c) which modifies
> in-place and uses default_pkg_ops.
>
> This seems rather misleading and fragile approach. Introduce a helper
> for the second use-case so that default_pkg_ops is handled consistently
> and essentially remains unchanged throughout.
>
> Signed-off-by: Emil Velikov <[email protected]>
> ---
> lib/libalpm/be_sync.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 225df76d..8fdf0fb2 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -281,6 +281,23 @@ static int _sync_get_validation(alpm_pkg_t *pkg)
> return pkg->validation;
> }
>
> +/** Package sync operations struct accessor. We implement this as a method
> + * rather than a static struct as in be_files because we want to reuse the
> + * majority of the default_pkg_ops struct and add only a few operations of
> + * our own on top.
This confused me a lot... What is be_files? Turns out the only
reference to be_files in the codebase is where this comment was copied
from in be_package.c!
So this comment needs fixed, and the one in be_package.c updated. I'll
handle both.
> + */
> +static struct pkg_operations *get_sync_pkg_ops(void)
> +{
> + static struct pkg_operations sync_pkg_ops;
> + static int sync_pkg_ops_initalized = 0;
> + if(!sync_pkg_ops_initalized) {
> + sync_pkg_ops = default_pkg_ops;
> + sync_pkg_ops.get_validation = _sync_get_validation;
> + sync_pkg_ops_initalized = 1;
> + }
> + return &sync_pkg_ops;
> +}
> +
> static alpm_pkg_t *load_pkg_for_entry(alpm_db_t *db, const char *entryname,
> const char **entry_filename, alpm_pkg_t *likely_pkg)
> {
> @@ -321,8 +338,7 @@ static alpm_pkg_t *load_pkg_for_entry(alpm_db_t *db,
> const char *entryname,
>
> pkg->origin = ALPM_PKG_FROM_SYNCDB;
> pkg->origin_data.db = db;
> - pkg->ops = &default_pkg_ops;
> - pkg->ops->get_validation = _sync_get_validation;
> + pkg->ops = get_sync_pkg_ops();
> pkg->handle = db->handle;
>
> /* add to the collection */
>
