Re: [pacman-dev] [PATCH] pacman+libalpm: handle search errors
On 6/11/19 11:42 am, morganamilo wrote:
> Previously, pacman treated no matches and an error during search the
> same.
>
> To fix this, alpm_db_search now returns its status as an int and
> instead takes the to be returned list as a param. Allowing front ends to
> easily differentiate between errors and no matches.
>
> Signed-off-by: morganamilo
>
Looks good to me. Only one bit I need to have a decent thing about,
but I think that it is fine.
Unless anyone else has a comment, I'll accept as is.
Allan
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 4486da44..d3630385 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -997,9 +997,11 @@ alpm_list_t *alpm_db_get_groupcache(alpm_db_t *db);
> /** Searches a database with regular expressions.
> * @param db pointer to the package database to search in
> * @param needles a list of regular expressions to search for
> - * @return the list of packages matching all regular expressions on
success, NULL on error
> + * @param ret the list of packages matching all regular expressions
> + * @return 0 on success, -1 on error (pm_errno is set accordingly)
> */> -alpm_list_t *alpm_db_search(alpm_db_t *db, const alpm_list_t
*needles);
> +int alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret);
OK
>
> typedef enum _alpm_db_usage_t {
> ALPM_DB_USAGE_SYNC = 1,
> diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
> index a443e552..cf4c865f 100644
> --- a/lib/libalpm/db.c
> +++ b/lib/libalpm/db.c
> @@ -298,12 +298,14 @@ alpm_list_t SYMEXPORT
*alpm_db_get_groupcache(alpm_db_t *db)
> }
>
> /** Searches a database. */
> -alpm_list_t SYMEXPORT *alpm_db_search(alpm_db_t *db, const
alpm_list_t *needles)
> +int SYMEXPORT alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret)
> {
> - ASSERT(db != NULL, return NULL);
> + ASSERT(db != NULL && ret != NULL && *ret == NULL,
> + RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1));
> db->handle->pm_errno = ALPM_ERR_OK;
>
> - return _alpm_db_search(db, needles);
> + return _alpm_db_search(db, needles, ret);
> }
>
OK
> /** Sets the usage bitmask for a repo */
> @@ -396,13 +398,13 @@ int _alpm_db_cmp(const void *d1, const void *d2)
> return strcmp(db1->treename, db2->treename);
> }
>
> -alpm_list_t *_alpm_db_search(alpm_db_t *db, const alpm_list_t *needles)
> +int _alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret)
> {
> const alpm_list_t *i, *j, *k;
> - alpm_list_t *ret = NULL;
>
> if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
> - return NULL;
> + return 0;
I had to think about this. Is trying to search a db that is labeled as
not being valid to search an error? That would require callers to check
database usage before searching. /This way, libalpm does the filtering.
I'm going to say this is fine...
> }
>
> /* copy the pkgcache- we will free the list var after each needle */
> @@ -415,12 +417,12 @@ alpm_list_t *_alpm_db_search(alpm_db_t *db,
const alpm_list_t *needles)
> if(i->data == NULL) {
> continue;
> }
> - ret = NULL;
> + *ret = NULL;
> targ = i->data;
> _alpm_log(db->handle, ALPM_LOG_DEBUG, "searching for target
'%s'\n", targ);
>
> if(regcomp(, targ, REG_EXTENDED | REG_NOSUB | REG_ICASE |
REG_NEWLINE) != 0) {
> - RET_ERR(db->handle, ALPM_ERR_INVALID_REGEX, NULL);
> + RET_ERR(db->handle, ALPM_ERR_INVALID_REGEX, -1);
> }
>
> for(j = list; j; j = j->next) {
OK
> @@ -463,18 +465,18 @@ alpm_list_t *_alpm_db_search(alpm_db_t *db,
const alpm_list_t *needles)
> _alpm_log(db->handle, ALPM_LOG_DEBUG,
> "search target '%s' matched
> '%s' on package '%s'\n",
> targ, matched, name);
> - ret = alpm_list_add(ret, pkg);
> + *ret = alpm_list_add(*ret, pkg);
> }
> }
>
> /* Free the existing search list, and use the returned list for
> the
>* next needle. This allows for AND-based package searching. */
> alpm_list_free(list);
> - list = ret;
> + list = *ret;
> regfree();
> }
>
> - return ret;
> + return 0;
> }
>
OK
> /* Returns a new package cache from db.
> diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h
> index f17444e6..e7ad98f5 100644
> --- a/lib/libalpm/db.h
> +++ b/lib/libalpm/db.h
> @@ -87,7 +87,8 @@ alpm_db_t *_alpm_db_new(const char *treename, int
is_local);
> void _alpm_db_free(alpm_db_t *db);
> const char *_alpm_db_path(alpm_db_t *db);
> int _alpm_db_cmp(const void *d1, const void
Re: [pacman-dev] [PATCH] pacman+libalpm: handle search errors
On Wed, 6 Nov 2019 at 01:44, morganamilo wrote:
>
> Previously, pacman treated no matches and an error during search the
> same.
>
> To fix this, alpm_db_search now returns its status as an int and
> instead takes the to be returned list as a param. Allowing front ends to
> easily differentiate between errors and no matches.
>
> Signed-off-by: morganamilo
>
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 4486da44..d3630385 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -997,9 +997,11 @@ alpm_list_t *alpm_db_get_groupcache(alpm_db_t *db);
> /** Searches a database with regular expressions.
> * @param db pointer to the package database to search in
> * @param needles a list of regular expressions to search for
> - * @return the list of packages matching all regular expressions on success,
> NULL on error
> + * @param ret the list of packages matching all regular expressions
> + * @return 0 on success, -1 on error (pm_errno is set accordingly)
> */
> -alpm_list_t *alpm_db_search(alpm_db_t *db, const alpm_list_t *needles);
> +int alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret);
>
> typedef enum _alpm_db_usage_t {
> ALPM_DB_USAGE_SYNC = 1,
> diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
> index a443e552..cf4c865f 100644
> --- a/lib/libalpm/db.c
> +++ b/lib/libalpm/db.c
> @@ -298,12 +298,14 @@ alpm_list_t SYMEXPORT *alpm_db_get_groupcache(alpm_db_t
> *db)
> }
>
> /** Searches a database. */
> -alpm_list_t SYMEXPORT *alpm_db_search(alpm_db_t *db, const alpm_list_t
> *needles)
> +int SYMEXPORT alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret)
> {
> - ASSERT(db != NULL, return NULL);
> + ASSERT(db != NULL && ret != NULL && *ret == NULL,
> + RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1));
> db->handle->pm_errno = ALPM_ERR_OK;
>
> - return _alpm_db_search(db, needles);
> + return _alpm_db_search(db, needles, ret);
> }
>
> /** Sets the usage bitmask for a repo */
> @@ -396,13 +398,13 @@ int _alpm_db_cmp(const void *d1, const void *d2)
> return strcmp(db1->treename, db2->treename);
> }
>
> -alpm_list_t *_alpm_db_search(alpm_db_t *db, const alpm_list_t *needles)
> +int _alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret)
> {
> const alpm_list_t *i, *j, *k;
> - alpm_list_t *ret = NULL;
>
> if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
> - return NULL;
> + return 0;
> }
>
> /* copy the pkgcache- we will free the list var after each needle */
> @@ -415,12 +417,12 @@ alpm_list_t *_alpm_db_search(alpm_db_t *db, const
> alpm_list_t *needles)
> if(i->data == NULL) {
> continue;
> }
> - ret = NULL;
> + *ret = NULL;
> targ = i->data;
> _alpm_log(db->handle, ALPM_LOG_DEBUG, "searching for target
> '%s'\n", targ);
>
> if(regcomp(, targ, REG_EXTENDED | REG_NOSUB | REG_ICASE |
> REG_NEWLINE) != 0) {
> - RET_ERR(db->handle, ALPM_ERR_INVALID_REGEX, NULL);
> + RET_ERR(db->handle, ALPM_ERR_INVALID_REGEX, -1);
> }
>
> for(j = list; j; j = j->next) {
> @@ -463,18 +465,18 @@ alpm_list_t *_alpm_db_search(alpm_db_t *db, const
> alpm_list_t *needles)
> _alpm_log(db->handle, ALPM_LOG_DEBUG,
> "search target '%s' matched
> '%s' on package '%s'\n",
> targ, matched, name);
> - ret = alpm_list_add(ret, pkg);
> + *ret = alpm_list_add(*ret, pkg);
> }
> }
>
> /* Free the existing search list, and use the returned list
> for the
> * next needle. This allows for AND-based package searching.
> */
> alpm_list_free(list);
> - list = ret;
> + list = *ret;
> regfree();
> }
>
> - return ret;
> + return 0;
> }
>
> /* Returns a new package cache from db.
> diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h
> index f17444e6..e7ad98f5 100644
> --- a/lib/libalpm/db.h
> +++ b/lib/libalpm/db.h
> @@ -87,7 +87,8 @@ alpm_db_t *_alpm_db_new(const char *treename, int is_local);
> void _alpm_db_free(alpm_db_t *db);
> const char *_alpm_db_path(alpm_db_t *db);
> int _alpm_db_cmp(const void *d1, const void *d2);
> -alpm_list_t *_alpm_db_search(alpm_db_t *db, const alpm_list_t *needles);
> +int _alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
> + alpm_list_t **ret);
> alpm_db_t *_alpm_db_register_local(alpm_handle_t *handle);
> alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle,
[pacman-dev] [PATCH] pacman+libalpm: handle search errors
Previously, pacman treated no matches and an error during search the
same.
To fix this, alpm_db_search now returns its status as an int and
instead takes the to be returned list as a param. Allowing front ends to
easily differentiate between errors and no matches.
Signed-off-by: morganamilo
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 4486da44..d3630385 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -997,9 +997,11 @@ alpm_list_t *alpm_db_get_groupcache(alpm_db_t *db);
/** Searches a database with regular expressions.
* @param db pointer to the package database to search in
* @param needles a list of regular expressions to search for
- * @return the list of packages matching all regular expressions on success,
NULL on error
+ * @param ret the list of packages matching all regular expressions
+ * @return 0 on success, -1 on error (pm_errno is set accordingly)
*/
-alpm_list_t *alpm_db_search(alpm_db_t *db, const alpm_list_t *needles);
+int alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
+ alpm_list_t **ret);
typedef enum _alpm_db_usage_t {
ALPM_DB_USAGE_SYNC = 1,
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index a443e552..cf4c865f 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -298,12 +298,14 @@ alpm_list_t SYMEXPORT *alpm_db_get_groupcache(alpm_db_t
*db)
}
/** Searches a database. */
-alpm_list_t SYMEXPORT *alpm_db_search(alpm_db_t *db, const alpm_list_t
*needles)
+int SYMEXPORT alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
+ alpm_list_t **ret)
{
- ASSERT(db != NULL, return NULL);
+ ASSERT(db != NULL && ret != NULL && *ret == NULL,
+ RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1));
db->handle->pm_errno = ALPM_ERR_OK;
- return _alpm_db_search(db, needles);
+ return _alpm_db_search(db, needles, ret);
}
/** Sets the usage bitmask for a repo */
@@ -396,13 +398,13 @@ int _alpm_db_cmp(const void *d1, const void *d2)
return strcmp(db1->treename, db2->treename);
}
-alpm_list_t *_alpm_db_search(alpm_db_t *db, const alpm_list_t *needles)
+int _alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
+ alpm_list_t **ret)
{
const alpm_list_t *i, *j, *k;
- alpm_list_t *ret = NULL;
if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
- return NULL;
+ return 0;
}
/* copy the pkgcache- we will free the list var after each needle */
@@ -415,12 +417,12 @@ alpm_list_t *_alpm_db_search(alpm_db_t *db, const
alpm_list_t *needles)
if(i->data == NULL) {
continue;
}
- ret = NULL;
+ *ret = NULL;
targ = i->data;
_alpm_log(db->handle, ALPM_LOG_DEBUG, "searching for target
'%s'\n", targ);
if(regcomp(, targ, REG_EXTENDED | REG_NOSUB | REG_ICASE |
REG_NEWLINE) != 0) {
- RET_ERR(db->handle, ALPM_ERR_INVALID_REGEX, NULL);
+ RET_ERR(db->handle, ALPM_ERR_INVALID_REGEX, -1);
}
for(j = list; j; j = j->next) {
@@ -463,18 +465,18 @@ alpm_list_t *_alpm_db_search(alpm_db_t *db, const
alpm_list_t *needles)
_alpm_log(db->handle, ALPM_LOG_DEBUG,
"search target '%s' matched
'%s' on package '%s'\n",
targ, matched, name);
- ret = alpm_list_add(ret, pkg);
+ *ret = alpm_list_add(*ret, pkg);
}
}
/* Free the existing search list, and use the returned list for
the
* next needle. This allows for AND-based package searching. */
alpm_list_free(list);
- list = ret;
+ list = *ret;
regfree();
}
- return ret;
+ return 0;
}
/* Returns a new package cache from db.
diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h
index f17444e6..e7ad98f5 100644
--- a/lib/libalpm/db.h
+++ b/lib/libalpm/db.h
@@ -87,7 +87,8 @@ alpm_db_t *_alpm_db_new(const char *treename, int is_local);
void _alpm_db_free(alpm_db_t *db);
const char *_alpm_db_path(alpm_db_t *db);
int _alpm_db_cmp(const void *d1, const void *d2);
-alpm_list_t *_alpm_db_search(alpm_db_t *db, const alpm_list_t *needles);
+int _alpm_db_search(alpm_db_t *db, const alpm_list_t *needles,
+ alpm_list_t **ret);
alpm_db_t *_alpm_db_register_local(alpm_handle_t *handle);
alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, const char *treename,
int level);
diff --git a/src/pacman/package.c b/src/pacman/package.c
index 35cfcd94..ec6e78fc 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -518,12 +518,13 @@ void print_groups(alpm_pkg_t *pkg)
* @param db the database
