(Work in progress) For now this only works for RPM, the others just return an empty string. --- generator/structs.ml | 1 + src/inspect-apps.c | 88 +++++++++++++++++++++++++++++----------------------- 2 files changed, 51 insertions(+), 38 deletions(-) diff --git a/generator/structs.ml b/generator/structs.ml index d62fcc5..8ac9fc5 100644 --- a/generator/structs.ml +++ b/generator/structs.ml @@ -183,6 +183,7 @@ let structs = [ "app_epoch", FInt32; "app_version", FString; "app_release", FString; + "app_arch", FString; "app_install_path", FString; "app_trans_path", FString; "app_publisher", FString; diff --git a/src/inspect-apps.c b/src/inspect-apps.c index f65c70a..3dbfb5b 100644 --- a/src/inspect-apps.c +++ b/src/inspect-apps.c @@ -31,6 +31,7 @@ #ifdef HAVE_ENDIAN_H #include <endian.h> #endif +#include <arpa/inet.h> #include <pcre.h> @@ -46,7 +47,7 @@ static struct guestfs_application_list *list_applications_rpm (guestfs_h *g, str #endif static struct guestfs_application_list *list_applications_deb (guestfs_h *g, struct inspect_fs *fs); static struct guestfs_application_list *list_applications_windows (guestfs_h *g, struct inspect_fs *fs); -static void add_application (guestfs_h *g, struct guestfs_application_list *, const char *name, const char *display_name, int32_t epoch, const char *version, const char *release, const char *install_path, const char *publisher, const char *url, const char *description); +static void add_application (guestfs_h *g, struct guestfs_application_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_application_list *); /* Unlike the simple inspect-get-* calls, this one assumes that the @@ -181,6 +182,37 @@ read_rpm_name (guestfs_h *g, return 0; } +/* tag constants, see rpmtag.h in RPM for complete list */ +#define RPMTAG_VERSION 1001 +#define RPMTAG_RELEASE 1002 +#define RPMTAG_ARCH 1022 + +static char * +get_rpm_header_tag (guestfs_h *g, const void *header_start, size_t header_len, uint32_t tag) +{ + uint32_t num_fields, offset; + const void *cursor = header_start + 8, *store; + + /* This function parses the RPM header structure to pull out various + * tag strings (version, release, arch, etc.). For more detail on the + * header format, see: + * http://www.rpm.org/max-rpm/s1-rpm-file-format-rpm-file-format.html#S2-RPM... + */ + + num_fields = ntohl (*(uint32_t *) header_start); + store = header_start + 8 + (16 * num_fields); + + while (cursor < store ){ + if (ntohl (*(uint32_t *) cursor) == tag){ + offset = ntohl(*(uint32_t *) (cursor + 8)); + return safe_strdup(g, store + offset); + } + cursor += 16; + } + + return NULL; +} + struct read_package_data { struct rpm_names_list *list; struct guestfs_application_list *apps; @@ -194,16 +226,13 @@ read_package (guestfs_h *g, { struct read_package_data *data = datav; struct rpm_name nkey, *entry; - char *p; - size_t len; - ssize_t max; - char *nul_name_nul, *version, *release; + char *version, *release, *arch; /* This function reads one (key, value) pair from the Packages * database. The key is the link field (see struct rpm_name). The - * value is a long binary string, but we can extract the version - * number from it as below. First we have to look up the link field - * in the list of links (which is sorted by link field). + * value is a long binary string, but we can extract the header data + * from it as below. First we have to look up the link field in the + * list of links (which is sorted by link field). */ /* Ignore bogus entries. */ @@ -219,41 +248,23 @@ read_package (guestfs_h *g, /* We found a matching link entry, so that gives us the application * name (entry->name). Now we can get other data for this - * application out of the binary value string. XXX This is a real - * hack. + * application out of the binary value string. */ - /* Look for \0<name>\0 */ - len = strlen (entry->name); - nul_name_nul = safe_malloc (g, len + 2); - nul_name_nul[0] = '\0'; - memcpy (&nul_name_nul[1], entry->name, len); - nul_name_nul[len+1] = '\0'; - p = memmem (value, valuelen, nul_name_nul, len+2); - free (nul_name_nul); - if (!p) - return 0; - - /* Following that are \0-delimited version and release fields. */ - p += len + 2; /* Note we have to skip \0 + name + \0. */ - max = valuelen - (p - (char *) value); - if (max < 0) - max = 0; - version = safe_strndup (g, p, max); - - len = strlen (version); - p += len + 1; - max = valuelen - (p - (char *) value); - if (max < 0) - max = 0; - release = safe_strndup (g, p, max); + version = get_rpm_header_tag(g, value, valuelen, RPMTAG_VERSION); + release = get_rpm_header_tag(g, value, valuelen, RPMTAG_RELEASE); + arch = get_rpm_header_tag(g, value, valuelen, RPMTAG_ARCH); /* Add the application and what we know. */ add_application (g, data->apps, entry->name, "", 0, version, release, - "", "", "", ""); + arch ? arch : "(none)", "", "", "", ""); free (version); free (release); + if (arch) + /* arch will occasionally be NULL, because some packages + * (e.g. "gpgkeys") do not have an arch */ + free (arch); return 0; } @@ -378,7 +389,7 @@ list_applications_deb (guestfs_h *g, struct inspect_fs *fs) else if (STREQ (line, "")) { if (installed_flag && name && version) add_application (g, apps, name, "", 0, version, release ? : "", - "", "", "", ""); + "", "", "", "", ""); free (name); free (version); free (release); @@ -519,7 +530,7 @@ list_applications_windows_from_path (guestfs_h *g, add_application (g, apps, name, display_name, 0, version ? : "", - "", + "", "", install_path ? : "", publisher ? : "", url ? : "", @@ -542,7 +553,7 @@ list_applications_windows_from_path (guestfs_h *g, static void add_application (guestfs_h *g, struct guestfs_application_list *apps, const char *name, const char *display_name, int32_t epoch, - const char *version, const char *release, + const char *version, const char *release, const char *arch, const char *install_path, const char *publisher, const char *url, const char *description) @@ -555,6 +566,7 @@ add_application (guestfs_h *g, struct guestfs_application_list *apps, apps->val[apps->len-1].app_epoch = epoch; apps->val[apps->len-1].app_version = safe_strdup (g, version); apps->val[apps->len-1].app_release = safe_strdup (g, release); + apps->val[apps->len-1].app_arch = safe_strdup (g, arch); apps->val[apps->len-1].app_install_path = safe_strdup (g, install_path); /* XXX Translated path is not implemented yet. */ apps->val[apps->len-1].app_trans_path = safe_strdup (g, ""); -- 1.7.11.4

On Tue, Oct 16, 2012 at 04:33:34PM -0400, John Eckersberg wrote: We still can't add app_arch directly to this structure, because it will change the size of the struct, breaking the ABI for existing callers. There are some alternatives, none great I'm afraid: (1) Reuse some existing field that would never be used by RPM (app_install_path? -- think this is used by Windows only). However this would stop us from adding arch support on Windows. (2) Create a new call inspect_list_applications2 and a new struct application2. (3) Same as (2), but disguise this using linker symbol versioning. This would be the best option but is rather complex to implement. However libvirt and glibc both provide examples of how to do it. In the meantime, one thing that strikes me about this patch is that it's split up wrong. There is a part of this patch which simplifies the parsing of the existing name and version fields. And there is another part which adds support for architecture (including adding it to virt-inspector). The first patch could be applied immediately, leaving the complex issue above to the second patch. The current split doesn't really make sense. safe_strndup I think? I now see what's confusing me about this patch .. it seems to be applied on top of the existing patches? Best to post a completely new series against the current HEAD. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

(Work in progress) For now this only works for RPM, the others just return an empty string. --- generator/actions.ml | 6 +++ generator/structs.ml | 1 + inspector/virt-inspector.c | 4 ++ src/inspect-apps.c | 111 +++++++++++++++++++++++++-------------------- 4 files changed, 73 insertions(+), 49 deletions(-) diff --git a/generator/actions.ml b/generator/actions.ml index 71aee37..3ad98fc 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1571,6 +1571,12 @@ The release string of the application or package, for package managers that use this. If unavailable this is returned as an empty string C<\"\">. +=item C<app_arch> + +The architecture string of the application or package, for package +managers that use this. If unavailable this is returned as an empty +string C<\"\">. + =item C<app_install_path> The installation path of the application (on operating systems diff --git a/generator/structs.ml b/generator/structs.ml index d62fcc5..8ac9fc5 100644 --- a/generator/structs.ml +++ b/generator/structs.ml @@ -183,6 +183,7 @@ let structs = [ "app_epoch", FInt32; "app_version", FString; "app_release", FString; + "app_arch", FString; "app_install_path", FString; "app_trans_path", FString; "app_publisher", FString; diff --git a/inspector/virt-inspector.c b/inspector/virt-inspector.c index 8b06fa7..3ac14a7 100644 --- a/inspector/virt-inspector.c +++ b/inspector/virt-inspector.c @@ -747,6 +747,10 @@ output_applications (xmlTextWriterPtr xo, char *root) XMLERROR (-1, xmlTextWriterWriteElement (xo, BAD_CAST "release", BAD_CAST apps->val[i].app_release)); + if (apps->val[i].app_arch && apps->val[i].app_arch[0]) + XMLERROR (-1, + xmlTextWriterWriteElement (xo, BAD_CAST "arch", + BAD_CAST apps->val[i].app_arch)); if (apps->val[i].app_install_path && apps->val[i].app_install_path[0]) XMLERROR (-1, xmlTextWriterWriteElement (xo, BAD_CAST "install_path", diff --git a/src/inspect-apps.c b/src/inspect-apps.c index f65c70a..e272513 100644 --- a/src/inspect-apps.c +++ b/src/inspect-apps.c @@ -46,7 +46,7 @@ static struct guestfs_application_list *list_applications_rpm (guestfs_h *g, str #endif static struct guestfs_application_list *list_applications_deb (guestfs_h *g, struct inspect_fs *fs); static struct guestfs_application_list *list_applications_windows (guestfs_h *g, struct inspect_fs *fs); -static void add_application (guestfs_h *g, struct guestfs_application_list *, const char *name, const char *display_name, int32_t epoch, const char *version, const char *release, const char *install_path, const char *publisher, const char *url, const char *description); +static void add_application (guestfs_h *g, struct guestfs_application_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_application_list *); /* Unlike the simple inspect-get-* calls, this one assumes that the @@ -125,7 +125,7 @@ guestfs__inspect_list_applications (guestfs_h *g, const char *root) #ifdef DB_DUMP /* This data comes from the Name database, and contains the application - * names and the first 4 bytes of the link field. + * names and the first 4 bytes of each link field. */ struct rpm_names_list { struct rpm_name *names; @@ -161,26 +161,62 @@ read_rpm_name (guestfs_h *g, void *listv) { struct rpm_names_list *list = listv; + const unsigned char *link_p; char *name; /* Ignore bogus entries. */ if (keylen == 0 || valuelen < 4) return 0; - /* The name (key) field won't be NUL-terminated, so we must do that. */ - name = safe_malloc (g, keylen+1); - memcpy (name, key, keylen); - name[keylen] = '\0'; - - list->names = safe_realloc (g, list->names, - (list->len + 1) * sizeof (struct rpm_name)); - list->names[list->len].name = name; - memcpy (list->names[list->len].link, value, 4); - list->len++; + /* A name entry will have as many links as installed instances of + * that pacakge. For example, if glibc.i686 and glibc.x86_64 are + * both installed, then there will be a link for each Packages + * entry. Add an entry onto list for all installed instances. + */ + for (link_p = value; link_p < value + valuelen; link_p += 8) { + name = safe_strndup (g, key, keylen); + + list->names = safe_realloc (g, list->names, + (list->len + 1) * sizeof (struct rpm_name)); + list->names[list->len].name = name; + memcpy (list->names[list->len].link, link_p, 4); + list->len++; + } return 0; } +/* tag constants, see rpmtag.h in RPM for complete list */ +#define RPMTAG_VERSION 1001 +#define RPMTAG_RELEASE 1002 +#define RPMTAG_ARCH 1022 + +static char * +get_rpm_header_tag (guestfs_h *g, const void *header_start, size_t header_len, uint32_t tag) +{ + uint32_t num_fields, offset; + const void *cursor = header_start + 8, *store; + + /* This function parses the RPM header structure to pull out various + * tag strings (version, release, arch, etc.). For more detail on the + * header format, see: + * http://www.rpm.org/max-rpm/s1-rpm-file-format-rpm-file-format.html#S2-RPM... + */ + + num_fields = be32toh (*(uint32_t *) header_start); + store = header_start + 8 + (16 * num_fields); + + while (cursor < store && cursor < header_start + header_len) { + if (be32toh (*(uint32_t *) cursor) == tag){ + offset = be32toh(*(uint32_t *) (cursor + 8)); + return safe_strdup(g, store + offset); + } + cursor += 16; + } + + return NULL; +} + struct read_package_data { struct rpm_names_list *list; struct guestfs_application_list *apps; @@ -194,16 +230,13 @@ read_package (guestfs_h *g, { struct read_package_data *data = datav; struct rpm_name nkey, *entry; - char *p; - size_t len; - ssize_t max; - char *nul_name_nul, *version, *release; + char *version, *release, *arch; /* This function reads one (key, value) pair from the Packages * database. The key is the link field (see struct rpm_name). The - * value is a long binary string, but we can extract the version - * number from it as below. First we have to look up the link field - * in the list of links (which is sorted by link field). + * value is a long binary string, but we can extract the header data + * from it as below. First we have to look up the link field in the + * list of links (which is sorted by link field). */ /* Ignore bogus entries. */ @@ -219,41 +252,20 @@ read_package (guestfs_h *g, /* We found a matching link entry, so that gives us the application * name (entry->name). Now we can get other data for this - * application out of the binary value string. XXX This is a real - * hack. + * application out of the binary value string. */ - /* Look for \0<name>\0 */ - len = strlen (entry->name); - nul_name_nul = safe_malloc (g, len + 2); - nul_name_nul[0] = '\0'; - memcpy (&nul_name_nul[1], entry->name, len); - nul_name_nul[len+1] = '\0'; - p = memmem (value, valuelen, nul_name_nul, len+2); - free (nul_name_nul); - if (!p) - return 0; - - /* Following that are \0-delimited version and release fields. */ - p += len + 2; /* Note we have to skip \0 + name + \0. */ - max = valuelen - (p - (char *) value); - if (max < 0) - max = 0; - version = safe_strndup (g, p, max); - - len = strlen (version); - p += len + 1; - max = valuelen - (p - (char *) value); - if (max < 0) - max = 0; - release = safe_strndup (g, p, max); + version = get_rpm_header_tag(g, value, valuelen, RPMTAG_VERSION); + release = get_rpm_header_tag(g, value, valuelen, RPMTAG_RELEASE); + arch = get_rpm_header_tag(g, value, valuelen, RPMTAG_ARCH); /* Add the application and what we know. */ add_application (g, data->apps, entry->name, "", 0, version, release, - "", "", "", ""); + arch ? arch : "", "", "", "", ""); free (version); free (release); + free (arch); return 0; } @@ -378,7 +390,7 @@ list_applications_deb (guestfs_h *g, struct inspect_fs *fs) else if (STREQ (line, "")) { if (installed_flag && name && version) add_application (g, apps, name, "", 0, version, release ? : "", - "", "", "", ""); + "", "", "", "", ""); free (name); free (version); free (release); @@ -519,7 +531,7 @@ list_applications_windows_from_path (guestfs_h *g, add_application (g, apps, name, display_name, 0, version ? : "", - "", + "", "", install_path ? : "", publisher ? : "", url ? : "", @@ -542,7 +554,7 @@ list_applications_windows_from_path (guestfs_h *g, static void add_application (guestfs_h *g, struct guestfs_application_list *apps, const char *name, const char *display_name, int32_t epoch, - const char *version, const char *release, + const char *version, const char *release, const char *arch, const char *install_path, const char *publisher, const char *url, const char *description) @@ -555,6 +567,7 @@ add_application (guestfs_h *g, struct guestfs_application_list *apps, apps->val[apps->len-1].app_epoch = epoch; apps->val[apps->len-1].app_version = safe_strdup (g, version); apps->val[apps->len-1].app_release = safe_strdup (g, release); + apps->val[apps->len-1].app_arch = safe_strdup (g, arch); apps->val[apps->len-1].app_install_path = safe_strdup (g, install_path); /* XXX Translated path is not implemented yet. */ apps->val[apps->len-1].app_trans_path = safe_strdup (g, ""); -- 1.7.11.7