[Libguestfs] [PATCH 1/2] filearch: move libmagic code in an own function

[Richard W.M. Jones]

On Mon, Apr 13, 2015 at 01:41:59PM +0200, Pino Toscano wrote:
> Also use a cleanup attribue to ease the close of the magic_t handle.
> 
> This is mostly code motion, hopefully with no actual behaviour changes.
> ---
>  src/filearch.c | 100 +++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 68 insertions(+), 32 deletions(-)
> 
> diff --git a/src/filearch.c b/src/filearch.c
> index 8708a4b..219eaae 100644
> --- a/src/filearch.c
> +++ b/src/filearch.c
> @@ -42,6 +42,22 @@
>  
>  #if defined(HAVE_LIBMAGIC)
>  
> +# ifdef HAVE_ATTRIBUTE_CLEANUP
> +# define CLEANUP_MAGIC_T_FREE __attribute__((cleanup(cleanup_magic_t_free)))
> +
> +static void
> +cleanup_magic_t_free (void *ptr)
> +{
> +  magic_t m = *(magic_t *) ptr;
> +
> +  if (m)
> +    magic_close (m);
> +}
> +
> +# else
> +# define CLEANUP_MAGIC_T_FREE
> +# endif
> +
>  COMPILE_REGEXP (re_file_elf,
>                  "ELF.*(?:executable|shared object|relocatable), (.+?),", 0)
>  COMPILE_REGEXP (re_elf_ppc64, "64.*PowerPC", 0)
> @@ -92,6 +108,55 @@ is_regular_file (const char *filename)
>    return lstat (filename, &statbuf) == 0 && S_ISREG (statbuf.st_mode);
>  }
>  
> +static char *
> +magic_for_file (guestfs_h *g, const char *filename, bool *loading_ok,
> +                bool *matched)
> +{
> +  int flags;
> +  CLEANUP_MAGIC_T_FREE magic_t m = NULL;
> +  const char *line;
> +  char *elf_arch;
> +
> +  flags = g->verbose ? MAGIC_DEBUG : 0;
> +  flags |= MAGIC_ERROR | MAGIC_RAW;
> +
> +  if (loading_ok)
> +    *loading_ok = false;
> +  if (matched)
> +    *matched = false;
> +
> +  m = magic_open (flags);
> +  if (m == NULL) {
> +    perrorf (g, "magic_open");
> +    return NULL;
> +  }
> +
> +  if (magic_load (m, NULL) == -1) {
> +    perrorf (g, "magic_load: default magic database file");
> +    return NULL;
> +  }
> +
> +  line = magic_file (m, filename);
> +  if (line == NULL) {
> +    perrorf (g, "magic_file: %s", filename);
> +    return NULL;
> +  }
> +
> +  if (loading_ok)
> +    *loading_ok = true;
> +
> +  elf_arch = match1 (g, line, re_file_elf);
> +  if (elf_arch == NULL) {
> +    error (g, "no re_file_elf match in '%s'", line);
> +    return NULL;
> +  }
> +
> +  if (matched)
> +    *matched = true;
> +
> +  return canonical_elf_arch (g, elf_arch);
> +}
> +
>  /* Download and uncompress the cpio file to find binaries within. */
>  static const char *initrd_binaries[] = {
>    "bin/ls",
> @@ -170,40 +235,11 @@ cpio_arch (guestfs_h *g, const char *file, const char *path)
>        safe_asprintf (g, "%s/%s", dir, initrd_binaries[i]);
>  
>      if (is_regular_file (bin)) {
> -      int flags;
> -      magic_t m;
> -      const char *line;
> -      CLEANUP_FREE char *elf_arch = NULL;
> -
> -      flags = g->verbose ? MAGIC_DEBUG : 0;
> -      flags |= MAGIC_ERROR | MAGIC_RAW;
> -
> -      m = magic_open (flags);
> -      if (m == NULL) {
> -        perrorf (g, "magic_open");
> -        goto out;
> -      }
> -
> -      if (magic_load (m, NULL) == -1) {
> -        perrorf (g, "magic_load: default magic database file");
> -        magic_close (m);
> -        goto out;
> -      }
> -
> -      line = magic_file (m, bin);
> -      if (line == NULL) {
> -        perrorf (g, "magic_file: %s", bin);
> -        magic_close (m);
> -        goto out;
> -      }
> +      bool loading_ok, matched;
>  
> -      elf_arch = match1 (g, line, re_file_elf);
> -      if (elf_arch != NULL) {
> -        ret = canonical_elf_arch (g, elf_arch);
> -        magic_close (m);
> +      ret = magic_for_file (g, bin, &loading_ok, &matched);
> +      if (!loading_ok || matched)
>          goto out;
> -      }
> -      magic_close (m);
>      }
>    }
>    error (g, "file_architecture: could not determine architecture of cpio archive");
> -- 
> 2.1.0

Yup looks like just code motion to me, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html