[pacman-dev] Thoughts on database initialization and loading process

[Rémy Oudompheng]

On Sun 17 April 2011 at 10:53 +1000, Allan McRae wrote:
> On 17/04/11 09:40, Dan McGee wrote:
> >4) Fix callsites everywhere that may force a load of the pkgcache to
> >actually check error return values for the "local db too old" and "gpg
> >sig check failed" cases, among other, rather than just assuming the
> >pkgcache is NULL without looking at pm_errno. Best solution as it
> >requires no API change, but will definitely have a large LOC impact.
> 
> I think this is the probably the best solution, not because lack of
> API change but because it seems the most correct thing to be doing.
> I also think we should do more return value checking in a lot of
> places in the code base...
> 

We are looking at a broader problem: it is actually impossible to do any
sane return value checking for all functions returning an alpm_list_t*
since NULL is used both as the error value and as the empty list.

[adding to this the fact that I am really not a fan of linked lists
especially when there doesn't seem to be any reason to use them.
NULL-terminated arrays of pointers work in almost all cases]

I suggest adding a special value to return when an error occurs.
Something like this: callers would check the value of pm_errno
whenever return value == pm_listerror.

I strongly support solving this return value problem (independently of
the DB signature checking protocol).

diff --git a/lib/libalpm/alpm_list.c b/lib/libalpm/alpm_list.c
index 0a3f650..501a33c 100644
--- a/lib/libalpm/alpm_list.c
+++ b/lib/libalpm/alpm_list.c
@@ -75,6 +75,9 @@ void SYMEXPORT alpm_list_free_inner(alpm_list_t *list, alpm_list_fn_free fn)
        }
 }
 
+/** Error value */
+static struct __alpm_list_t _alpm_list_dummy = { NULL, NULL, NULL };
+alpm_list_t * const pm_listerror SYMEXPORT = &_alpm_list_dummy;
 
 /* Mutators */
 
diff --git a/lib/libalpm/alpm_list.h b/lib/libalpm/alpm_list.h
index 1f6393a..6fb7fde 100644
--- a/lib/libalpm/alpm_list.h
+++ b/lib/libalpm/alpm_list.h
@@ -42,6 +42,9 @@ typedef struct __alpm_list_t {
        struct __alpm_list_t *next;
 } alpm_list_t;
 
+/** Return value when error occurs. */
+extern alpm_list_t * const pm_listerror;
+
 #define FREELIST(p) do { alpm_list_free_inner(p, free); alpm_list_free(p); p = NULL; } while(0)
 
 typedef void (*alpm_list_fn_free)(void *); /* item deallocation callback */
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index b55b02a..4939926 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -269,7 +269,7 @@ alpm_list_t SYMEXPORT *alpm_option_get_syncdbs()
 {
        if (handle == NULL) {
                pm_errno = PM_ERR_HANDLE_NULL;
-               return NULL;
+               return pm_listerror;
        }
        return handle->dbs_sync;
 }


-- 
Rémy.