Re: [Libguestfs] [PATCH] list-applications: Add support for pacman

On Sun, Nov 16, 2014 at 03:24:16PM +0200, Nikos Skalkotos wrote: > Extend the guestfs_inspect_list_applications2 API call to work on Arch > Linux guest images. Generally looks good. I have a few minor comments inline below. But also I think we could use a test case (see tests/guests/). I don't think we'd reject the patch for not having a test case, but the test case would ensure it doesn't break or start misbehaving in future, so it's good for Arch. > src/inspect-apps.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 125 insertions(+) > > diff --git a/src/inspect-apps.c b/src/inspect-apps.c > index b62b432..b7a3b0e 100644 > --- a/src/inspect-apps.c > +++ b/src/inspect-apps.c > @@ -60,6 +60,7 @@ > static struct guestfs_application2_list *list_applications_rpm (guestfs_h *g, struct inspect_fs *fs); > #endif > static struct guestfs_application2_list *list_applications_deb (guestfs_h *g, struct inspect_fs *fs); > +static struct guestfs_application2_list *list_applications_pacman (guestfs_h *g, struct inspect_fs *fs); > static struct guestfs_application2_list *list_applications_windows (guestfs_h *g, struct inspect_fs *fs); > static void add_application (guestfs_h *g, struct guestfs_application2_list *, const char *name, const char *display_name, int32_t epoch, const char *version, const char *release, const char *arch, const char *install_path, const char *publisher, const char *url, const char *description); > static void sort_applications (struct guestfs_application2_list *); > @@ -145,6 +146,11 @@ guestfs__inspect_list_applications2 (guestfs_h *g, const char *root) > break; > > case OS_PACKAGE_FORMAT_PACMAN: > + ret = list_applications_pacman (g, fs); > + if (ret == NULL) > + return NULL; > + break; > + > case OS_PACKAGE_FORMAT_EBUILD: > case OS_PACKAGE_FORMAT_PISI: > case OS_PACKAGE_FORMAT_PKGSRC: > @@ -494,6 +500,125 @@ list_applications_deb (guestfs_h *g, struct inspect_fs *fs) > return ret; > } > > +static struct guestfs_application2_list * > +list_applications_pacman (guestfs_h *g, struct inspect_fs *fs) > +{ > + CLEANUP_FREE char *desc_file = NULL, *fname = NULL; > + struct guestfs_application2_list *apps = NULL, *ret = NULL; > + struct guestfs_dirent *curr = NULL; > + FILE *fp; > + char line[1024]; > + size_t i, len; > + CLEANUP_FREE char *name = NULL, *version = NULL, *desc = NULL; > + CLEANUP_FREE char *arch = NULL, *url = NULL; > + char *release = NULL; > + char **key = NULL; > + const size_t path_len = strlen ("/var/lib/pacman/local/") + strlen ("/desc"); > + struct guestfs_dirent_list *local_db = NULL; > + > + local_db = guestfs_readdir (g, "/var/lib/pacman/local"); > + if (local_db == NULL) > + return NULL; > + > + /* Allocate 'apps' list. */ > + apps = safe_malloc (g, sizeof *apps); > + apps->len = 0; > + apps->val = NULL; > + > + Extra blank line here. > + for (i = 0; i < local_db->len; i++) { > + curr = &local_db->val[i]; > + > + if (curr->ftyp != 'd' || STREQ (curr->name, ".") || STREQ (curr->name, "..")) > + continue; > + > + free (fname); > + fname = safe_malloc (g, strlen (curr->name) + path_len + 1); > + sprintf (fname, "/var/lib/pacman/local/%s/desc", curr->name); > + free (desc_file); > + desc_file = guestfs___download_to_tmp (g, fs, fname, curr->name, 8192); > + > + /* The desc files are small (4K). If the desc file does not exist, or is > + * larger than the 8K limit we've used, the database is probably corrupted, > + * but we'll continue with the next package anyway. > + */ > + if (desc_file == NULL) > + continue; > + > + fp = fopen (desc_file, "r"); > + if (fp == NULL) { > + perrorf (g, "fopen: %s", desc_file); > + goto out; > + } > + > + while (fgets (line, sizeof line, fp) != NULL) { This might be cleaner if it used getline(3). I notice that we don't use getline in the src/ directory at the moment. However we do use it elsewhere, and we import it from gnulib if it's not provided by libc. It's also easier to use than fgets / static buffers. > + len = strlen (line); > + if (len > 0 && line[len-1] == '\n') { > + line[--len] = '\0'; > + } > + > + /* empty line */ > + if (len == 0) { > + key = NULL; > + continue; > + } > + > + if (key != NULL) { > + *key = safe_strdup (g, line); > + key = NULL; > + continue; > + } > + > + if (STREQ(line, "%NAME%")) > + key = &name; > + else if (STREQ(line, "%VERSION%")) > + key = &version; > + else if (STREQ(line, "%DESC%")) > + key = &desc; > + else if (STREQ(line, "%URL%")) > + key = &url; > + else if (STREQ(line, "%ARCH%")) > + key = &arch; > + } > + > + if (name) { > + if (version) { > + char *p = strchr (version, '-'); > + if (p) { > + *p = '\0'; > + release = p + 1; > + } > + } > + add_application (g, apps, name, "", 0, version ? : "", release ? : "", > + arch ? : "", "", "", url ? : "", desc ? : ""); > + } > + > + free (name); > + free (version); > + free (desc); > + free (arch); > + free (url); > + name = version = desc = arch = url = NULL; > + release = NULL; /* Haven't allocated memory for release */ > + > + if (fclose (fp) == -1) { > + perrorf (g, "fclose: %s", desc_file); > + goto out; > + } > + > + } > + > + ret = apps; > + > + out: > + guestfs_free_dirent_list (local_db); Maybe use CLEANUP_FREE_DIRENT_LIST instead of having to call guestfs_free_dirent_list along the exit path? > + > + if (ret == NULL) > + guestfs_free_application2_list (apps); > + > + return ret; > +} > + > static void list_applications_windows_from_path (guestfs_h *g, struct guestfs_application2_list *apps, const char **path, size_t path_len); > > static struct guestfs_application2_list * Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v