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.