[pacman-dev] [PATCH 2/6] Hook new optdepend structures up
Dan McGee
dpmcgee at gmail.com
Thu Jul 21 15:30:25 EDT 2011
On Thu, Jul 21, 2011 at 1:39 PM, Benedikt Morbach
<benedikt.morbach at googlemail.com> wrote:
> No new behaviour introduced, everything should work exactly as before.
> ---
> lib/libalpm/be_local.c | 11 ++++++++-
> lib/libalpm/be_package.c | 5 ++-
> lib/libalpm/be_sync.c | 7 +++++-
> lib/libalpm/package.c | 7 ++++-
> src/pacman/package.c | 11 ++++++++-
> src/pacman/util.c | 51 ++++++++++++++++++++++++++++++++++++----------
> 6 files changed, 72 insertions(+), 20 deletions(-)
>
> diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
> index 70f242d..7e4812b 100644
> --- a/lib/libalpm/be_local.c
> +++ b/lib/libalpm/be_local.c
> @@ -610,7 +610,12 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq)
> info->depends = alpm_list_add(info->depends, _alpm_splitdep(line));
> }
> } else if(strcmp(line, "%OPTDEPENDS%") == 0) {
> - READ_AND_STORE_ALL(info->optdepends);
> + /* Different than the rest because of the _alpm_splitoptdep call. */
> + while(1) {
> + READ_NEXT();
> + if(strlen(line) == 0) break;
> + info->optdepends = alpm_list_add(info->optdepends, _alpm_splitoptdep(line));
> + }
> } else if(strcmp(line, "%CONFLICTS%") == 0) {
> READ_AND_STORE_ALL(info->conflicts);
> } else if(strcmp(line, "%PROVIDES%") == 0) {
> @@ -800,7 +805,9 @@ int _alpm_local_db_write(alpm_db_t *db, alpm_pkg_t *info, alpm_dbinfrq_t inforeq
> if(info->optdepends) {
> fputs("%OPTDEPENDS%\n", fp);
> for(lp = info->optdepends; lp; lp = lp->next) {
> - fprintf(fp, "%s\n", (char *)lp->data);
> + char *optstring = alpm_optdep_compute_string(lp->data);
> + fprintf(fp, "%s\n", optstring);
> + free(optstring);
> }
> fprintf(fp, "\n");
> }
> diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
> index 46bdaed..438960c 100644
> --- a/lib/libalpm/be_package.c
> +++ b/lib/libalpm/be_package.c
> @@ -35,7 +35,7 @@
> #include "log.h"
> #include "handle.h"
> #include "package.h"
> -#include "deps.h" /* _alpm_splitdep */
> +#include "deps.h" /* _alpm_splitdep _alpm_splitoptdep */
Use a comma if you update this comment, but really you can just kill
the comment bit completely.
>
> /**
> * Open a package changelog for reading. Similar to fopen in functionality,
> @@ -192,7 +192,8 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t *
> alpm_depend_t *dep = _alpm_splitdep(ptr);
> newpkg->depends = alpm_list_add(newpkg->depends, dep);
> } else if(strcmp(key, "optdepend") == 0) {
> - newpkg->optdepends = alpm_list_add(newpkg->optdepends, strdup(ptr));
> + alpm_optdepend_t *optdep = _alpm_splitoptdep(ptr);
> + newpkg->optdepends = alpm_list_add(newpkg->optdepends, optdep);
> } else if(strcmp(key, "conflict") == 0) {
> newpkg->conflicts = alpm_list_add(newpkg->conflicts, strdup(ptr));
> } else if(strcmp(key, "replaces") == 0) {
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index daed01f..3a0c2b4 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -555,7 +555,12 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive,
> pkg->depends = alpm_list_add(pkg->depends, _alpm_splitdep(line));
> }
> } else if(strcmp(line, "%OPTDEPENDS%") == 0) {
> - READ_AND_STORE_ALL(pkg->optdepends);
> + /* Different than the rest because of the _alpm_splitoptdep call. */
> + while(1) {
> + READ_NEXT();
> + if(strlen(line) == 0) break;
> + pkg->optdepends = alpm_list_add(pkg->optdepends, _alpm_splitoptdep(line));
> + }
> } else if(strcmp(line, "%CONFLICTS%") == 0) {
> READ_AND_STORE_ALL(pkg->conflicts);
> } else if(strcmp(line, "%PROVIDES%") == 0) {
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index ae9b9a9..a64d613 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -502,7 +502,9 @@ alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg)
> for(i = pkg->depends; i; i = alpm_list_next(i)) {
> newpkg->depends = alpm_list_add(newpkg->depends, _alpm_dep_dup(i->data));
> }
> - newpkg->optdepends = alpm_list_strdup(pkg->optdepends);
> + for(i = pkg->optdepends; i; i = alpm_list_next(i)) {
> + newpkg->optdepends = alpm_list_add(newpkg->optdepends, _alpm_optdep_dup(i->data));
> + }
> newpkg->conflicts = alpm_list_strdup(pkg->conflicts);
> newpkg->provides = alpm_list_strdup(pkg->provides);
> for(i = pkg->deltas; i; i = alpm_list_next(i)) {
> @@ -551,7 +553,8 @@ void _alpm_pkg_free(alpm_pkg_t *pkg)
> alpm_list_free(pkg->backup);
> alpm_list_free_inner(pkg->depends, (alpm_list_fn_free)_alpm_dep_free);
> alpm_list_free(pkg->depends);
> - FREELIST(pkg->optdepends);
> + alpm_list_free_inner(pkg->optdepends, (alpm_list_fn_free)_alpm_optdep_free);
> + alpm_list_free(pkg->optdepends);
> FREELIST(pkg->conflicts);
> FREELIST(pkg->provides);
> alpm_list_free_inner(pkg->deltas, (alpm_list_fn_free)_alpm_delta_free);
> diff --git a/src/pacman/package.c b/src/pacman/package.c
> index afbac6b..6e6d379 100644
> --- a/src/pacman/package.c
> +++ b/src/pacman/package.c
> @@ -53,7 +53,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra)
> const char *label;
> double size;
> const alpm_list_t *i;
> - alpm_list_t *requiredby = NULL, *depstrings = NULL;
> + alpm_list_t *depstrings = NULL, *optstrings = NULL, *requiredby = NULL;
>
> if(pkg == NULL) {
> return;
> @@ -87,6 +87,12 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra)
> depstrings = alpm_list_add(depstrings, alpm_dep_compute_string(dep));
> }
>
> + /* turn optdepends list into a text list */
> + for(i = alpm_pkg_get_optdepends(pkg); i; i = alpm_list_next(i)) {
> + alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i);
> + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep));
> + }
> +
> if(extra || from == PKG_FROM_LOCALDB) {
> /* compute this here so we don't get a pause in the middle of output */
> requiredby = alpm_pkg_compute_requiredby(pkg);
> @@ -104,7 +110,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra)
> list_display(_("Groups :"), alpm_pkg_get_groups(pkg));
> list_display(_("Provides :"), alpm_pkg_get_provides(pkg));
> list_display(_("Depends On :"), depstrings);
> - list_display_linebreak(_("Optional Deps :"), alpm_pkg_get_optdepends(pkg));
> + list_display_linebreak(_("Optional Deps :"), optstrings);
> if(extra || from == PKG_FROM_LOCALDB) {
> list_display(_("Required By :"), requiredby);
> }
> @@ -158,6 +164,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra)
> printf("\n");
>
> FREELIST(depstrings);
> + FREELIST(optstrings);
> FREELIST(requiredby);
> }
>
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 7065abd..14dcf94 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -974,32 +974,61 @@ void print_packages(const alpm_list_t *packages)
> }
> }
>
> -/* Helper function for comparing strings using the
> +/* Helper function for comparing optdepends using the
> * alpm "compare func" signature */
> -int str_cmp(const void *s1, const void *s2)
> +int opt_cmp(const void *o1, const void *o2)
> {
> - return strcmp(s1, s2);
> + char *str1 = alpm_optdep_compute_string((alpm_optdepend_t*)o1);
> + char *str2 = alpm_optdep_compute_string((alpm_optdepend_t*)o2);
> + int ret = strcmp(str1, str2);
> +
> + free(str1);
> + free(str2);
> +
> + return ret;
Something feels a bit off with this logic. Wouldn't we really just
want to compare by the name portion? Even if we want to compare by
more, we can just do it with three fallthrough strcmp()s which is a
bit better than having to compute and free these strings over and
over. e.g.:
int ret;
alpm od *od1 = o1;
alpm_od *od2 = o2;
ret = strcmp(od1->name, od2->name);
if(!ret && od1->version && od2->version) {
ret = strcmp(od1->version, od2->version);
}
if(!ret && od1->desc && od2->desc) {
ret = strcmp(od1->desc, od2->desc);
}
return ret;
> }
>
> void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg)
> {
> - alpm_list_t *old = alpm_pkg_get_optdepends(oldpkg);
> - alpm_list_t *new = alpm_pkg_get_optdepends(newpkg);
> - alpm_list_t *optdeps = alpm_list_diff(new,old,str_cmp);
> - if(optdeps) {
> + alpm_list_t *i, *old, *new, *optdeps, *optstrings = NULL;
> +
> + old = alpm_pkg_get_optdepends(oldpkg);
> + new = alpm_pkg_get_optdepends(newpkg);
> + optdeps = alpm_list_diff(new,old,opt_cmp);
Since we're changing this line, can you fix the coding style and use
spaces after commas please?
> +
> + /* turn optdepends list into a text list */
> + for(i = optdeps; i; i = alpm_list_next(i)) {
> + alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i);
No need for the cast here, it is implied in C.
> + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep));
> + }
> +
> + if(optstrings) {
> printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg));
> - list_display_linebreak(" ", optdeps);
> + list_display_linebreak(" ", optstrings);
> }
> +
> alpm_list_free(optdeps);
> + FREELIST(optstrings);
> }
>
> void display_optdepends(alpm_pkg_t *pkg)
> {
> - alpm_list_t *optdeps = alpm_pkg_get_optdepends(pkg);
> - if(optdeps) {
> + alpm_list_t *i, *optdeps, *optstrings = NULL;
> +
> + optdeps = alpm_pkg_get_optdepends(pkg);
> +
> + /* turn optdepends list into a text list */
> + for(i = optdeps; i; i = alpm_list_next(i)) {
> + alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i);
Same as above, no need for casting.
> + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep));
> + }
> +
> + if(optstrings) {
> printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg));
> - list_display_linebreak(" ", optdeps);
> + list_display_linebreak(" ", optstrings);
> }
> +
> + FREELIST(optstrings);
> }
>
> static void display_repo_list(const char *dbname, alpm_list_t *list)
> --
> 1.7.6
>
>
>
More information about the pacman-dev
mailing list