[PATCH v4 0/2] inspector: recognize ppc64 and ppc64le archs (RHBZ#1211996)
Patch extracts MSB/LSB info from /bin/file output and passes it as separate parameret from elf_arch. Then it is sent to impl_file_architecture and checked/ Modified magic_for_file to handle regex changes. Now with tests for file_architecture. Fixes: RHBZ#1211996 Maros Zatko (2): inspector: recognize ppc64 and ppc64le archs (RHBZ#1211996) tests: ppc64 and ppc64le for file_architecture generator/actions.ml | 8 ++++++++ src/filearch.c | 24 +++++++++++++++--------- 2 files changed, 23 insertions(+), 9 deletions(-) -- 1.9.3
--- src/filearch.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/filearch.c b/src/filearch.c index 8fb1acf..ce9d0a1 100644 --- a/src/filearch.c +++ b/src/filearch.c @@ -59,14 +59,14 @@ 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, ".*64.*PowerPC", 0) /* Convert output from 'file' command on ELF files to the canonical * architecture string. Caller must free the result. */ static char * -canonical_elf_arch (guestfs_h *g, const char *elf_arch) +canonical_elf_arch (guestfs_h *g, const char *endianness, const char *elf_arch) { const char *r; char *ret; @@ -85,8 +85,12 @@ canonical_elf_arch (guestfs_h *g, const char *elf_arch) r = "sparc64"; else if (strstr (elf_arch, "IA-64")) r = "ia64"; - else if (match (g, elf_arch, re_elf_ppc64)) - r = "ppc64"; + else if (match (g, elf_arch, re_elf_ppc64)) { + if (strstr (endianness, "MSB")) + r = "ppc64"; + else if (strstr (endianness, "LSB")) + r = "ppc64le"; + } else if (strstr (elf_arch, "PowerPC")) r = "ppc"; else if (strstr (elf_arch, "ARM aarch64")) @@ -116,6 +120,7 @@ magic_for_file (guestfs_h *g, const char *filename, bool *loading_ok, CLEANUP_MAGIC_T_FREE magic_t m = NULL; const char *line; CLEANUP_FREE char *elf_arch = NULL; + CLEANUP_FREE char *endianness = NULL; flags = g->verbose ? MAGIC_DEBUG : 0; flags |= MAGIC_ERROR | MAGIC_RAW; @@ -145,7 +150,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, &endianness, &elf_arch); if (elf_arch == NULL) { error (g, "no re_file_elf match in '%s'", line); return NULL; @@ -154,7 +159,7 @@ magic_for_file (guestfs_h *g, const char *filename, bool *loading_ok, if (matched) *matched = true; - return canonical_elf_arch (g, elf_arch); + return canonical_elf_arch (g, endianness, elf_arch); } /* Download and uncompress the cpio file to find binaries within. */ @@ -315,6 +320,7 @@ 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; char *ret = NULL; /* Get the output of the "file" command. Note that because this @@ -324,8 +330,8 @@ 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) + ret = canonical_elf_arch (g, endianness, elf_arch); else if (strstr (file, "PE32 executable")) ret = safe_strdup (g, "i386"); else if (strstr (file, "PE32+ executable")) -- 1.9.3
On Tuesday 19 May 2015 20:23:37 Maros Zatko wrote:
--- src/filearch.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/filearch.c b/src/filearch.c index 8fb1acf..ce9d0a1 100644 --- a/src/filearch.c +++ b/src/filearch.c @@ -59,14 +59,14 @@ 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, ".*64.*PowerPC", 0)
/* Convert output from 'file' command on ELF files to the canonical * architecture string. Caller must free the result. */ static char * -canonical_elf_arch (guestfs_h *g, const char *elf_arch) +canonical_elf_arch (guestfs_h *g, const char *endianness, const char *elf_arch) { const char *r; char *ret; @@ -85,8 +85,12 @@ canonical_elf_arch (guestfs_h *g, const char *elf_arch) r = "sparc64"; else if (strstr (elf_arch, "IA-64")) r = "ia64"; - else if (match (g, elf_arch, re_elf_ppc64)) - r = "ppc64"; + else if (match (g, elf_arch, re_elf_ppc64)) { + if (strstr (endianness, "MSB")) + r = "ppc64"; + else if (strstr (endianness, "LSB")) + r = "ppc64le";
Can you please add an else branch signalling with error() the unknown endianness string and returning NULL? Just a small safe guard. -- Pino Toscano
relates to RHBZ#1211996 --- generator/actions.ml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/generator/actions.ml b/generator/actions.ml index 1a89869..cf3d525 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -788,6 +788,10 @@ to specify the QEMU interface emulation to use at run time." }; InitISOFS, Always, TestResultString ( [["file_architecture"; "/bin-i586-dynamic"]], "i386"), []; InitISOFS, Always, TestResultString ( + [["file_architecture"; "/bin-ppc64-dynamic"]], "ppc64"), []; + InitISOFS, Always, TestResultString ( + [["file_architecture"; "/bin-ppc64le-dynamic"]], "ppc64le"), []; + InitISOFS, Always, TestResultString ( [["file_architecture"; "/bin-sparc-dynamic"]], "sparc"), []; InitISOFS, Always, TestResultString ( [["file_architecture"; "/bin-win32.exe"]], "i386"), []; @@ -802,6 +806,10 @@ to specify the QEMU interface emulation to use at run time." }; InitISOFS, Always, TestResultString ( [["file_architecture"; "/lib-i586.so"]], "i386"), []; InitISOFS, Always, TestResultString ( + [["file_architecture"; "/lib-ppc64.so"]], "ppc64"), []; + InitISOFS, Always, TestResultString ( + [["file_architecture"; "/lib-ppc64le.so"]], "ppc64le"), []; + InitISOFS, Always, TestResultString ( [["file_architecture"; "/lib-sparc.so"]], "sparc"), []; InitISOFS, Always, TestResultString ( [["file_architecture"; "/lib-win32.dll"]], "i386"), []; -- 1.9.3
On Tuesday 19 May 2015 20:23:36 Maros Zatko wrote:
Patch extracts MSB/LSB info from /bin/file output and passes it as separate parameret from elf_arch. Then it is sent to impl_file_architecture and checked/
Modified magic_for_file to handle regex changes.
Now with tests for file_architecture.
Fixes: RHBZ#1211996
Maros Zatko (2): inspector: recognize ppc64 and ppc64le archs (RHBZ#1211996) tests: ppc64 and ppc64le for file_architecture
Except one small note in patch 1, it seems generally good. One thing though, could you please merge the tests into the main commit, so it's more clear they go together? Thanks for the work, -- Pino Toscano
participants (2)
-
Maros Zatko -
Pino Toscano