[Libguestfs] [PATCH 1/2] Updates to original patch, following feedback.

[Richard W.M. Jones]

On Tue, Oct 16, 2012 at 04:33:34PM -0400, John Eckersberg wrote:
> ---
>  generator/actions.ml |  6 ++++++
>  src/inspect-apps.c   | 46 +++++++++++++++++++++++++---------------------
>  2 files changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/generator/actions.ml b/generator/actions.ml
> index dc71234..afafe9b 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -1527,6 +1527,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<\"\">.
> +

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.

> +    name = safe_malloc (g, keylen+1);
> +    memcpy (name, key, keylen);
> +    name[keylen] = '\0';

safe_strndup I think?

> -  num_fields = ntohl (*(uint32_t *) header_start);
> +  num_fields = be32toh (*(uint32_t *) header_start);

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