Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   ArchLinux Pacman Development (http://www.linux-archive.org/archlinux-pacman-development/)
-   -   Remove need to explicitly register the local DB (http://www.linux-archive.org/archlinux-pacman-development/482622-remove-need-explicitly-register-local-db.html)

Dan McGee 01-28-2011 10:48 PM

Remove need to explicitly register the local DB
 
There is rarely a reason to ever not set up the local DB when getting a
libalpm handle, so just do it automatically at this time. It is still a lazy
initialization anyway, so there should be little to no fallout from doing
it this way.

Signed-off-by: Dan McGee <dan@archlinux.org>
---
lib/libalpm/alpm.c | 4 ++++
lib/libalpm/alpm.h | 2 --
lib/libalpm/be_local.c | 7 +------
lib/libalpm/db.c | 15 ---------------
src/pacman/database.c | 4 ++--
src/pacman/pacman.c | 9 ---------
src/pacman/query.c | 11 +++++++++--
src/pacman/remove.c | 2 --
src/pacman/sync.c | 11 ++++++-----
src/util/pactree.c | 2 +-
src/util/testdb.c | 2 +-
11 files changed, 24 insertions(+), 45 deletions(-)

diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
index 2a9f460..7c3bfc2 100644
--- a/lib/libalpm/alpm.c
+++ b/lib/libalpm/alpm.c
@@ -54,6 +54,10 @@ int SYMEXPORT alpm_initialize(void)
if(handle == NULL) {
RET_ERR(PM_ERR_MEMORY, -1);
}
+ if(_alpm_db_register_local() == NULL) {
+ /* error code should be set */
+ return(-1);
+ }

#ifdef ENABLE_NLS
bindtextdomain("libalpm", LOCALEDIR);
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index a540bc4..95482f0 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -174,8 +174,6 @@ typedef enum _pmpkgreason_t {
* Databases
*/

-/* Preferred interfaces db_register_local and db_register_sync */
-pmdb_t *alpm_db_register_local(void);
pmdb_t *alpm_db_register_sync(const char *treename);
int alpm_db_unregister(pmdb_t *db);
int alpm_db_unregister_all(void);
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index ea59cec..9602c82 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -400,7 +400,7 @@ static int local_db_populate(pmdb_t *db)
pkg = _alpm_pkg_new();
if(pkg == NULL) {
closedir(dbdir);
- return(-1);
+ RET_ERR(PM_ERR_MEMORY, -1);
}
/* split the db entry name */
if(_alpm_splitname(name, pkg) != 0) {
@@ -900,11 +900,6 @@ pmdb_t *_alpm_db_register_local(void)

ALPM_LOG_FUNC;

- if(handle->db_local != NULL) {
- _alpm_log(PM_LOG_WARNING, _("attempt to re-register the 'local' DB
"));
- RET_ERR(PM_ERR_DB_NOT_NULL, NULL);
- }
-
_alpm_log(PM_LOG_DEBUG, "registering local database
");

db = _alpm_db_new("local", 1);
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index bf9a70d..c80dcbb 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -64,21 +64,6 @@ pmdb_t SYMEXPORT *alpm_db_register_sync(const char *treename)
return(_alpm_db_register_sync(treename));
}

-/** Register the local package database.
- * @return a pmdb_t* representing the local database, or NULL on error
- */
-pmdb_t SYMEXPORT *alpm_db_register_local(void)
-{
- ALPM_LOG_FUNC;
-
- /* Sanity checks */
- ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, NULL));
- /* Do not register a database if a transaction is on-going */
- ASSERT(handle->trans == NULL, RET_ERR(PM_ERR_TRANS_NOT_NULL, NULL));
-
- return(_alpm_db_register_local());
-}
-
/* Helper function for alpm_db_unregister{_all} */
void _alpm_db_unregister(pmdb_t *db)
{
diff --git a/src/pacman/database.c b/src/pacman/database.c
index 5fd33ea..36433f3 100644
--- a/src/pacman/database.c
+++ b/src/pacman/database.c
@@ -31,8 +31,6 @@
#include "conf.h"
#include "util.h"

-extern pmdb_t *db_local;
-
/**
* @brief Modify the 'local' package database.
*
@@ -43,6 +41,7 @@ extern pmdb_t *db_local;
int pacman_database(alpm_list_t *targets)
{
alpm_list_t *i;
+ pmdb_t *db_local;
int retval = 0;
pmpkgreason_t reason;

@@ -65,6 +64,7 @@ int pacman_database(alpm_list_t *targets)
return(1);
}

+ db_local = alpm_option_get_localdb();
for(i = targets; i; i = alpm_list_next(i)) {
char *pkgname = i->data;
if(alpm_db_set_pkgreason(db_local, pkgname, reason) == -1) {
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index c267060..45500cf 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -57,7 +57,6 @@
#include "conf.h"
#include "package.h"

-pmdb_t *db_local;
/* list of targets specified on command line */
static alpm_list_t *pm_targets;

@@ -1433,14 +1432,6 @@ int main(int argc, char *argv[])
list_display("Targets :", pm_targets);
}

- /* Opening local database */
- db_local = alpm_db_register_local();
- if(db_local == NULL) {
- pm_printf(PM_LOG_ERROR, _("could not register 'local' database (%s)
"),
- alpm_strerrorlast());
- cleanup(EXIT_FAILURE);
- }
-
/* Log commandline */
if(needs_root()) {
cl_to_log(argc, argv);
diff --git a/src/pacman/query.c b/src/pacman/query.c
index fc6a2a5..c79133d 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -37,8 +37,6 @@
#include "conf.h"
#include "util.h"

-extern pmdb_t *db_local;
-
static char *resolve_path(const char *file)
{
char *str = NULL;
@@ -102,6 +100,7 @@ static int query_fileowner(alpm_list_t *targets)
char *append;
size_t max_length;
alpm_list_t *t;
+ pmdb_t *db_local;

/* This code is here for safety only */
if(targets == NULL) {
@@ -117,6 +116,8 @@ static int query_fileowner(alpm_list_t *targets)
append = path + strlen(path);
max_length = PATH_MAX - (append - path) - 1;

+ db_local = alpm_option_get_localdb();
+
for(t = targets; t; t = alpm_list_next(t)) {
char *filename, *dname, *rpath;
const char *bname;
@@ -220,6 +221,7 @@ static int query_search(alpm_list_t *targets)
{
alpm_list_t *i, *searchlist;
int freelist;
+ pmdb_t *db_local = alpm_option_get_localdb();

/* if we have a targets list, search for packages matching it */
if(targets) {
@@ -286,6 +288,8 @@ static int query_group(alpm_list_t *targets)
alpm_list_t *i, *j;
char *grpname = NULL;
int ret = 0;
+ pmdb_t *db_local = alpm_option_get_localdb();
+
if(targets == NULL) {
for(j = alpm_db_get_grpcache(db_local); j; j = alpm_list_next(j)) {
pmgrp_t *grp = alpm_list_getdata(j);
@@ -471,6 +475,7 @@ int pacman_query(alpm_list_t *targets)
int match = 0;
alpm_list_t *i;
pmpkg_t *pkg = NULL;
+ pmdb_t *db_local;

/* First: operations that do not require targets */

@@ -495,6 +500,8 @@ int pacman_query(alpm_list_t *targets)
}
}

+ db_local = alpm_option_get_localdb();
+
/* operations on all packages in the local DB
* valid: no-op (plain -Q), list, info, check
* invalid: isfile, owns */
diff --git a/src/pacman/remove.c b/src/pacman/remove.c
index 52f92ec..82d1c38 100644
--- a/src/pacman/remove.c
+++ b/src/pacman/remove.c
@@ -31,8 +31,6 @@
#include "util.h"
#include "conf.h"

-extern pmdb_t *db_local;
-
/**
* @brief Remove a specified list of packages.
*
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 278f15e..7353f7e 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -37,8 +37,6 @@
#include "package.h"
#include "conf.h"

-extern pmdb_t *db_local;
-
/* if keep_used != 0, then the db files which match an used syncdb
* will be kept */
static int sync_cleandb(const char *dbpath, int keep_used) {
@@ -144,6 +142,7 @@ static int sync_cleancache(int level)
{
alpm_list_t *i;
alpm_list_t *sync_dbs = alpm_option_get_syncdbs();
+ pmdb_t *db_local = alpm_option_get_localdb();
int ret = 0;

for(i = alpm_option_get_cachedirs(); i; i = alpm_list_next(i)) {
@@ -295,7 +294,7 @@ static int sync_synctree(int level, alpm_list_t *syncs)
return(success > 0);
}

-static void print_installed(pmpkg_t *pkg)
+static void print_installed(pmdb_t *db_local, pmpkg_t *pkg)
{
const char *pkgname = alpm_pkg_get_name(pkg);
const char *pkgver = alpm_pkg_get_version(pkg);
@@ -316,6 +315,7 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)
alpm_list_t *i, *j, *ret;
int freelist;
int found = 0;
+ pmdb_t *db_local = alpm_option_get_localdb();

for(i = syncs; i; i = alpm_list_next(i)) {
pmdb_t *db = alpm_list_getdata(i);
@@ -366,7 +366,7 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)
printf(")");
}

- print_installed(pkg);
+ print_installed(db_local, pkg);

/* we need a newline and initial indent first */
printf("
");
@@ -519,6 +519,7 @@ static int sync_info(alpm_list_t *syncs, alpm_list_t *targets)
static int sync_list(alpm_list_t *syncs, alpm_list_t *targets)
{
alpm_list_t *i, *j, *ls = NULL;
+ pmdb_t *db_local = alpm_option_get_localdb();

if(targets) {
for(i = targets; i; i = alpm_list_next(i)) {
@@ -556,7 +557,7 @@ static int sync_list(alpm_list_t *syncs, alpm_list_t *targets)
if (!config->quiet) {
printf("%s %s %s", alpm_db_get_name(db), alpm_pkg_get_name(pkg),
alpm_pkg_get_version(pkg));
- print_installed(pkg);
+ print_installed(db_local, pkg);
printf("
");
} else {
printf("%s
", alpm_pkg_get_name(pkg));
diff --git a/src/util/pactree.c b/src/util/pactree.c
index 0ac3f24..6a10006 100644
--- a/src/util/pactree.c
+++ b/src/util/pactree.c
@@ -77,7 +77,7 @@ static int alpm_local_init(void)
return(ret);
}

- db_local = alpm_db_register_local();
+ db_local = alpm_option_get_localdb();
if(!db_local) {
return(1);
}
diff --git a/src/util/testdb.c b/src/util/testdb.c
index 96a123a..461cf23 100644
--- a/src/util/testdb.c
+++ b/src/util/testdb.c
@@ -136,7 +136,7 @@ static int check_localdb(void) {
return(ret);
}

- db = alpm_db_register_local();
+ db = alpm_option_get_localdb();
if(db == NULL) {
fprintf(stderr, "error: could not register 'local' database (%s)
",
alpm_strerrorlast());
--
1.7.3.5

Xyne 01-29-2011 03:51 AM

Remove need to explicitly register the local DB
 
Dan McGee wrote:

> There is rarely a reason to ever not set up the local DB when getting a
> libalpm handle, so just do it automatically at this time. It is still a lazy
> initialization anyway, so there should be little to no fallout from doing
> it this way.

I'm concerned about the uncertainty of "little to no fallout". What exactly
does the "lazy" initialization do? Does it in any way rely on the presence of
a local database?

I ask because I have a tool in mind that will use ALPM to query the sync
database, most likely in the absence of a local database.

It also strikes me as bad design to hard-code something that is not necessary
for all operations, especially for a library that is at least intended for use
beyond Pacman. Wouldn't it make more sense to create a separate routine to
automatically set up the local DB when getting a libalpm handle? Applications
that need the local DB could just use that instead wherever they acquire the
handle.

Allan McRae 01-29-2011 06:35 AM

Remove need to explicitly register the local DB
 
On 29/01/11 14:51, Xyne wrote:

Dan McGee wrote:


There is rarely a reason to ever not set up the local DB when getting a
libalpm handle, so just do it automatically at this time. It is still a lazy
initialization anyway, so there should be little to no fallout from doing
it this way.


I'm concerned about the uncertainty of "little to no fallout". What exactly
does the "lazy" initialization do? Does it in any way rely on the presence of
a local database?


The lazy initialisation means that nothing is actually loaded when
registering the local database. The local database is only read in as
needed and only the parts that are needed are read in.


In fact, all that is done registering the local database is creating a
pmdb_t srtuct and assigning the local database operation struct to it.
That is the "little to no fallout" if you do not use the local database.


Allan

Xyne 01-29-2011 08:09 AM

Remove need to explicitly register the local DB
 
Allan McRae wrote:

> The lazy initialisation means that nothing is actually loaded when
> registering the local database. The local database is only read in as
> needed and only the parts that are needed are read in.
>
> In fact, all that is done registering the local database is creating a
> pmdb_t srtuct and assigning the local database operation struct to it.
> That is the "little to no fallout" if you do not use the local database.
>
> Allan
>

Thanks for the clarification.

Even if it is negligible, it's munging separate logic together. Given that the
purpose is presumably to save coders a few lines of code, I still think it
would make more sense to use a wrapper function to do that when getting the
handle.

Don't worry though, I'll leave it there as I have no intention of pestering you
about something that is effectively irrelevant. I just see it as lazy design in
a bad way.

Regards,
Xyne

Xyne 01-30-2011 03:46 PM

Remove need to explicitly register the local DB
 
Dan McGee wrote:

> The "register" code could just as well be done in the handle setup
> where I moved the call anyway. You'd save about 13 CPU cycles. It's
> design that is meant to remove stupidity in a library where you have
> to make what seems like a no-op call- but make sure you only do it
> once. Why let users shoot themselves in the foot? We already handled
> the teardown in alpm_release so this just makes the initialize side
> map that.


Xavier Chantry wrote:

> It's not separate logic. Dan commit log might have been a bit misleading.
>
> alpm_initialize initializes (wow) whatever is needed for the other
> functions to work. What we do there is just allocate some structures
> like the handle, and now handle->db_local as well. We are talking
> about one calloc and one strdup here to setup db_local. Really no big
> deal, there is no loading of the db, not even a check that one exists
> at all.
> And it does make it simpler for libalpm users. And as Dan said its
> symetric with alpm_release.
>


Ah. It makes much more sense now, especially given the symmetry with
alpm_release. Thanks.




> > -Dan
> >
> > -Dan
> >
>
> ^^ A sign I need my morning coffee... :)

I just assumed that you had graduated to Double Dan:
https://secure.wikimedia.org/wikipedia/en/wiki/Dan_%28rank%29


All times are GMT. The time now is 07:45 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.