Date: Sun, 11 Aug 2013 09:01:31 GMT From: mattbw@FreeBSD.org To: svn-soc-all@FreeBSD.org Subject: socsvn commit: r255800 - in soc2013/mattbw/backend: . query Message-ID: <201308110901.r7B91V8X046248@socsvn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mattbw Date: Sun Aug 11 09:01:31 2013 New Revision: 255800 URL: http://svnweb.FreeBSD.org/socsvn/?view=rev&rev=255800 Log: Clean up query code a bit. After a week of mostly shuffling around jobs, query/core.c has now received a bit of love. The initial query routing to local and remote querying has received the most changes, mainly because it was horrifically duplicated code previously. It's becoming obvious that query/core.c might need splitting, as it is now getting unfathomably long. Some space/indent errors elsewhere were fixed, but the code is long overdue an indent(1). Modified: soc2013/mattbw/backend/jobs.c soc2013/mattbw/backend/pkgutils.c soc2013/mattbw/backend/query/core.c soc2013/mattbw/backend/query/do.c Modified: soc2013/mattbw/backend/jobs.c ============================================================================== --- soc2013/mattbw/backend/jobs.c Sun Aug 11 08:38:10 2013 (r255799) +++ soc2013/mattbw/backend/jobs.c Sun Aug 11 09:01:31 2013 (r255800) @@ -131,17 +131,17 @@ jobs_emit_packages(struct pkg_jobs *jobs, PkBackend *backend, pkg_info_ptr info) { - struct pkg *pkg; + struct pkg *pkg; - assert(jobs != NULL); - assert(backend != NULL); - assert(info != NULL); - - pkg = NULL; - while (pkg_jobs(jobs, &pkg) == EPKG_OK) { - assert(pkg != NULL); - pkgutils_emit(pkg, backend, info(pkg)); - } + assert(jobs != NULL); + assert(backend != NULL); + assert(info != NULL); + + pkg = NULL; + while (pkg_jobs(jobs, &pkg) == EPKG_OK) { + assert(pkg != NULL); + pkgutils_emit(pkg, backend, info(pkg)); + } } void @@ -210,5 +210,3 @@ return namevers; } - - Modified: soc2013/mattbw/backend/pkgutils.c ============================================================================== --- soc2013/mattbw/backend/pkgutils.c Sun Aug 11 08:38:10 2013 (r255799) +++ soc2013/mattbw/backend/pkgutils.c Sun Aug 11 09:01:31 2013 (r255800) @@ -27,6 +27,7 @@ #include "pkgutils.h" /* Prototypes */ #include "utils.h" /* INTENTIONALLY_IGNORE */ +static bool pkg_matches_filters(struct pkg *pkg, PkBitfield filters); static const char *repo_of_remote(struct pkg *pkg); /* @@ -36,7 +37,7 @@ pkgutils_pkg_current_state(struct pkg *pkg) { PkInfoEnum info; - + assert(pkg != NULL); /* If the package is local, then it's installed. If it is remote, then @@ -178,7 +179,7 @@ */ gchar * pkgutils_pkg_match_id(struct pkg *pkg, gchar **split_id) -{ +{ bool success; int i; gchar *match_id; @@ -192,37 +193,45 @@ match_id = NULL; pkg_id_bits = calloc(4, sizeof(const char *)); - if (pkg_id_bits == NULL) - goto cleanup; + if (pkg_id_bits != NULL) { + match_id = pkgutils_pkg_to_id_through(pkg, pkg_id_bits); + } - match_id = pkgutils_pkg_to_id_through(pkg, pkg_id_bits); - if (match_id == NULL) - goto cleanup; + if (match_id != NULL) { + bool match_failed; - /* - * Succeed if this package's PackageID fields match the - * original PackageID. Of course, the original ID might have - * missing fields (NULLs), so we treat a comparison involving - * one as a success. This means using our "weak strcmp" - * instead of normal strcmp or even g_strcmp0. - */ - for (success = true, i = PK_PACKAGE_ID_NAME; - success && i <= PK_PACKAGE_ID_DATA; - i++) - success = string_match(split_id[i], pkg_id_bits[i]); - -cleanup: - if (!success) { - if (match_id != NULL) { + assert(pkg_id_bits != NULL); + + /* + * Succeed if this package's PackageID fields match the + * original PackageID. Of course, the original ID might have + * missing fields (NULLs), so we treat a comparison involving + * one as a success. This means using our "weak strcmp" + * instead of normal strcmp or even g_strcmp0. + */ + match_failed = false; + for (i = PK_PACKAGE_ID_NAME; i <= PK_PACKAGE_ID_DATA; i++) { + if (!string_match(split_id[i], pkg_id_bits[i])) { + match_failed = true; + break; + } + } + + if (match_failed) { g_free(match_id); match_id = NULL; + } else { + success = true; } + } + + free(pkg_id_bits); + assert (success || match_id == NULL); assert (!success || match_id != NULL); /* The strings inside this are owned by 'pkg'. Don't free them */ - free(pkg_id_bits); return match_id; } @@ -339,31 +348,45 @@ pkgutils_emit_filtered(struct pkg *pkg, PkBackend *backend, PkBitfield filters, PkInfoEnum info) { - gboolean should_emit; assert(pkg != NULL); assert(backend != NULL); - should_emit = TRUE; + if (pkg_matches_filters(pkg, filters)) { + pkgutils_emit(pkg, backend, info); + } +} + +static bool +pkg_matches_filters(struct pkg *pkg, PkBitfield filters) +{ + bool matches_filters; + PkFilterEnum wrong_filter; + + assert(pkg != NULL); + + matches_filters = true; + + if (pkgutils_pkg_current_state(pkg) == PK_INFO_ENUM_INSTALLED) { + wrong_filter = PK_FILTER_ENUM_NOT_INSTALLED; + } else { + wrong_filter = PK_FILTER_ENUM_INSTALLED; + } - if (pkgutils_pkg_current_state(pkg) == PK_INFO_ENUM_INSTALLED) - should_emit = pk_bitfield_contain(filters, - PK_FILTER_ENUM_NOT_INSTALLED) ? FALSE : should_emit; - else - should_emit = pk_bitfield_contain(filters, - PK_FILTER_ENUM_INSTALLED) ? FALSE : should_emit; - /* TODO: more filters? */ - if (should_emit) - pkgutils_emit(pkg, backend, info); + if (pk_bitfield_contain(filters, wrong_filter) == TRUE) { + matches_filters = false; + } + + return matches_filters; } /* * Gets the PackageKit repository name for the (remote) package. - * + * * Currently this is actually the pkgng repository ident. This might change. - * + * * This does not need to be freed (possibly, TODO: check). */ static const char * Modified: soc2013/mattbw/backend/query/core.c ============================================================================== --- soc2013/mattbw/backend/query/core.c Sun Aug 11 08:38:10 2013 (r255799) +++ soc2013/mattbw/backend/query/core.c Sun Aug 11 09:01:31 2013 (r255800) @@ -50,11 +50,17 @@ struct query_target *t; }; +typedef struct pkgdb_it * (*query_func_ptr) (struct query *q); + static bool can_remote_iterate(struct query *q); static enum repo_type type_of_repo_name(const char *name); static gchar *match_pkg(struct pkg *pkg, struct query *q); static gchar **init_unpack_source(PkBackend *backend, struct query_source *s); +static gchar **make_fake_id_split(const char *name); static struct pkg *match_iterator(struct pkgdb_it *it, struct query *q, gchar **match_id_p); +static struct pkg *query_find_from(query_func_ptr function, struct query *q, gchar **match_id_p); +static struct pkgdb_it *local_query(struct query *q); +static struct pkgdb_it *remote_query(struct query *q); /* * Returns the backend stored inside the given query. @@ -78,24 +84,15 @@ bool success; bool try_local; bool try_remote; - match_t match; PkBitfield filters; - const char *name; - const char *repo; gchar *match_id; struct pkg *pkg; - struct pkgdb *db; - struct pkgdb_it *it; assert(q != NULL); success = false; - match = MATCH_EXACT; match_id = NULL; pkg = NULL; - db = q->db; - name = q->id_strv[PK_PACKAGE_ID_NAME]; - repo = query_repo(q); /* * If we're not given a specific repository in the PackageID, we want @@ -112,35 +109,54 @@ if (pk_bitfield_contain(filters, PK_FILTER_ENUM_NOT_INSTALLED)) try_local = false; - /* Try a local search first, if applicable. */ - it = (try_local ? pkgdb_query(db, name, match) : NULL); - if (it != NULL) - pkg = match_iterator(it, q, &match_id); - pkgdb_it_free(it); - - /* No point trying remote if we got a local match */ - if (pkg != NULL) - try_remote = false; - - /* Next, try a remote search, again only if applicable. */ - it = (try_remote ? pkgdb_rquery(db, name, match, repo) : NULL); - if (it != NULL && can_remote_iterate(q)) - pkg = match_iterator(it, q, &match_id); - pkgdb_it_free(it); + if (try_local) { + pkg = query_find_from(local_query, q, &match_id); + } + if (pkg == NULL && try_remote) { + pkg = query_find_from(remote_query, q, &match_id); + } + if (pkg == NULL) { + assert(match_id == NULL); - if (pkg == NULL) ERR(q->backend, PK_ERROR_ENUM_PACKAGE_NOT_FOUND, "package not found"); - else - success = q->t->f(pkg, match_id, q); + } else { + assert(match_id != NULL); - pkg_free(pkg); - g_free(match_id); + success = q->t->f(pkg, match_id, q); + pkg_free(pkg); + g_free(match_id); + } return success; } +static struct pkg * +query_find_from(query_func_ptr function, struct query *q, gchar **match_id_p) +{ + struct pkg *result; + struct pkgdb_it *iterator; + + assert(function != NULL); + assert(q != NULL); + assert(match_id_p != NULL); + + result = NULL; + + iterator = function(q); + if (iterator != NULL) { + result = match_iterator(iterator, q, match_id_p); + pkgdb_it_free(iterator); + } + + /* Match ID if and only if result. */ + assert(result == NULL || *(match_id_p) != NULL); + assert(result != NULL || *(match_id_p) == NULL); + + return result; +} + /* * Retrieves the repository for the query, or NULL if none is specified (or * the query is using the local database only). @@ -148,13 +164,13 @@ const char * query_repo(struct query *q) { - + assert(q != NULL); return (q->rtype == REPO_REMOTE ? q->id_strv[PK_PACKAGE_ID_DATA] : NULL); } - + /* * Returns the database stored inside the given query. */ @@ -170,7 +186,7 @@ /* * Creates a struct query for the given backend, database, source and target. - * + * * Usually you'll want to use "query_do" instead of calling this directly. */ struct query * @@ -251,11 +267,12 @@ assert(q != NULL); - if (percent == PK_BACKEND_PERCENTAGE_INVALID) + if (percent == PK_BACKEND_PERCENTAGE_INVALID) { scaled_percent = PK_BACKEND_PERCENTAGE_INVALID; - else { - if (percent > 100) + } else { + if (100 < percent) { percent = 100; + } scaled_percent = (((100 * q->s->position) + percent) / (q->s->total)); @@ -324,8 +341,6 @@ assert(pkg != NULL); assert(q != NULL); - - /* No need to do a PackageID match if we're looking for a name. */ if (q->s->type == QUERY_SINGLE_NAME) match_id = pkgutils_pkg_to_id(pkg); @@ -367,38 +382,78 @@ return pkg; } +static struct pkgdb_it * +local_query(struct query *q) +{ + + return pkgdb_query(q->db, q->id_strv[PK_PACKAGE_ID_NAME], MATCH_EXACT); +} + +static struct pkgdb_it * +remote_query(struct query *q) +{ + struct pkgdb_it *iterator; + + iterator = pkgdb_rquery(q->db, q->id_strv[PK_PACKAGE_ID_NAME], + MATCH_EXACT, query_repo(q)); + + /* + * Make sure we can use this iterator. (We only check now so that + * the case of there being no remote matches is properly handled.) + */ + if (iterator != NULL && !can_remote_iterate(q)) { + pkgdb_it_free(iterator); + iterator = NULL; + } + + return iterator; +} + /* Unpacks a query source into name/version/arch/data pointers. */ static gchar ** init_unpack_source(PkBackend *backend, struct query_source *s) { - gchar **id_strv; + gchar **id_strv; assert(backend != NULL); assert(s != NULL); id_strv = NULL; if (s->type == QUERY_SINGLE_NAME) { - /* - * Make a snake-oil ID vector; we won't be doing rigid - * checking against it so it doesn't matter that the version - * might be in the name column etc. - */ - id_strv = g_malloc0_n(5, (gsize) sizeof(gchar *)); - - id_strv[PK_PACKAGE_ID_NAME] = g_strdup(s->single); - id_strv[PK_PACKAGE_ID_VERSION] = g_strdup(""); - id_strv[PK_PACKAGE_ID_ARCH] = g_strdup(""); - id_strv[PK_PACKAGE_ID_DATA] = g_strdup(""); + id_strv = make_fake_id_split(s->single); } else if (s->type == QUERY_SINGLE_ID) { id_strv = pk_package_id_split(s->single); if (id_strv == NULL) ERR(backend, PK_ERROR_ENUM_PACKAGE_ID_INVALID, "invalid package id"); - } else + } else { ERR(backend, PK_ERROR_ENUM_INTERNAL_ERROR, "init_unpack_source given silly source type"); + } + + return id_strv; +} + +static gchar ** +make_fake_id_split(const char *name) +{ + gchar **id_strv; + + assert(name != NULL); + + /* + * Make a snake-oil ID vector; we won't be doing rigid + * checking against it so it doesn't matter that the version + * might be in the name column etc. + */ + id_strv = g_malloc0_n(5, (gsize) sizeof(gchar *)); + + id_strv[PK_PACKAGE_ID_NAME] = g_strdup(name); + id_strv[PK_PACKAGE_ID_VERSION] = g_strdup(""); + id_strv[PK_PACKAGE_ID_ARCH] = g_strdup(""); + id_strv[PK_PACKAGE_ID_DATA] = g_strdup(""); return id_strv; } Modified: soc2013/mattbw/backend/query/do.c ============================================================================== --- soc2013/mattbw/backend/query/do.c Sun Aug 11 08:38:10 2013 (r255799) +++ soc2013/mattbw/backend/query/do.c Sun Aug 11 09:01:31 2013 (r255800) @@ -86,6 +86,10 @@ for (new_s.position = 0; new_s.position < new_s.total && success; (new_s.position)++) { + if (!success) { + break; + } + new_s.single = package_ids[new_s.position]; /* Treat non-PackageIDs as pkgng package names, if allowed */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201308110901.r7B91V8X046248>