[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]