[Libguestfs] [PATCH v2] inspector: recognize ppc64 and ppc64le archs (RHBZ#1211996)

Pino Toscano ptoscano at redhat.com
Fri Apr 17 08:56:29 UTC 2015


On Thursday 16 April 2015 16:51:48 Maros Zatko wrote:
> ---
>  src/filearch.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/src/filearch.c b/src/filearch.c
> index df65c98..5de2959 100644
> --- a/src/filearch.c
> +++ b/src/filearch.c
> @@ -59,8 +59,9 @@ cleanup_magic_t_free (void *ptr)
>  # endif
>  
>  COMPILE_REGEXP (re_file_elf,
> -                "ELF.*(?:executable|shared object|relocatable), (.+?),", 0)
> -COMPILE_REGEXP (re_elf_ppc64, "64.*PowerPC", 0)
> +                "ELF.*(MSB|LSB).*(?:executable|shared object|relocatable), (.+?),", 0)
> +COMPILE_REGEXP (re_elf_ppc64, "MSB.*64.*PowerPC", 0)
> +COMPILE_REGEXP (re_elf_ppc64le, "LSB.*64.*PowerPC", 0)
>  
>  /* Convert output from 'file' command on ELF files to the canonical
>   * architecture string.  Caller must free the result.
> @@ -87,6 +88,8 @@ canonical_elf_arch (guestfs_h *g, const char *elf_arch)
>      r = "ia64";
>    else if (match (g, elf_arch, re_elf_ppc64))
>      r = "ppc64";
> +  else if (match (g, elf_arch, re_elf_ppc64le))
> +    r = "ppc64le";
>    else if (strstr (elf_arch, "PowerPC"))
>      r = "ppc";
>    else if (strstr (elf_arch, "ARM aarch64"))
> @@ -114,6 +117,7 @@ magic_for_file (guestfs_h *g, const char *filename, bool *loading_ok,
>  {
>    int flags;
>    CLEANUP_MAGIC_T_FREE magic_t m = NULL;
> +  CLEANUP_FREE char *tmp = NULL;
>    const char *line;
>    char *elf_arch;
>  
> @@ -145,7 +149,7 @@ magic_for_file (guestfs_h *g, const char *filename, bool *loading_ok,
>    if (loading_ok)
>      *loading_ok = true;
>  
> -  elf_arch = match1 (g, line, re_file_elf);
> +  match2 (g, line, re_file_elf, &tmp, &elf_arch);
>    if (elf_arch == NULL) {

Better switch the checking from elf_arch (which is not touched if
match2 fails, so being an uninitialized pointer) to the return value
of match2.

>      error (g, "no re_file_elf match in '%s'", line);
>      return NULL;
> @@ -315,6 +319,8 @@ guestfs_impl_file_architecture (guestfs_h *g, const char *path)
>  {
>    CLEANUP_FREE char *file = NULL;
>    CLEANUP_FREE char *elf_arch = NULL;
> +  CLEANUP_FREE char *endianness = NULL;
> +  CLEANUP_FREE char *end_elf_arch = NULL;
>    char *ret = NULL;
>  
>    /* Get the output of the "file" command.  Note that because this
> @@ -324,8 +330,10 @@ guestfs_impl_file_architecture (guestfs_h *g, const char *path)
>    if (file == NULL)
>      return NULL;
>  
> -  if ((elf_arch = match1 (g, file, re_file_elf)) != NULL)
> -    ret = canonical_elf_arch (g, elf_arch);
> +  if ((match2 (g, file, re_file_elf, &endianness, &elf_arch)) != 0) {
> +    end_elf_arch = guestfs_int_safe_asprintf (g, "%s %s", endianness, elf_arch);
> +    ret = canonical_elf_arch (g, end_elf_arch);
> +  }

I still do think this approach, i.e. match MSB|LSB, create a new string
with it and have it matched again, is doing the same thing twice for no
reason.

As I suggested in the v1 review, please do pass the result of the
MSB|LSB match as new parameter for canonical_elf_arch, checking it
when the architecture is ppc64. This will also avoid having a new regex,
re_elf_ppc64le, and leave re_elf_ppc64 as it is currently:

char *
canonical_elf_arch (guestfs_h *g, const char *endianness,
                    const char *elf_arch)
{
  ...
    else if (match (g, elf_arch, re_elf_ppc64)) {
      if (STREQ (endianness, "LSB")
        r = "ppc64le";
      else
        r = "ppc64";
    }
}

No need for a new regex, and reuses the new MSB|LSB match as it is.

-- 
Pino Toscano




More information about the Libguestfs mailing list