[Libguestfs] [PATCH] list-applications: Add support for pacman
Nikos Skalkotos
skalkoto at gmail.com
Mon Nov 17 11:41:28 UTC 2014
OK, I'll make the suggested changes and I'll try to come up with a new
patch by tomorrow or the day after tomorrow. I just noticed that a
space is missing between STREQ and ( in the key assignment code which
violates the project's coding style. I'll fix that too.
For a test-case, I can write a make-archlinux-img.sh script and send
it in another patch. It's not big deal.
Another thing, I left out of the patch the epoch translation. Pacman's
version formats looks like this: epoch:version-rel. You have the epoch
hard-coded to 0 in list_applications_deb() and
list_applications_rpm(), so I did the same but I can implement it for
the sake of completeness. Should I be using the
guestfs___parse_unsigned_int() function for the string to int
conversion?
Nikos
On 17 November 2014 00:16, Richard W.M. Jones <rjones at redhat.com> wrote:
> 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
More information about the Libguestfs
mailing list