[1/6] mach-o: Don't split version into a few fields

Message ID 20181106140637.78145-2-r.bolshakov@yadro.com
State Superseded
Headers show
Series
  • Fixes for new mach-o load commands
Related show

Commit Message

Roman Bolshakov Nov. 6, 2018, 2:06 p.m.
X.Y.Z tuple is used for minos version and sdk version fields in a number of
load commands:
  LC_VERSION_MIN_MACOSX
  LC_VERSION_MIN_IOS
  LC_VERSION_MIN_WATCHOS
  LC_VERSION_MIN_TVOS
  LC_BUILD_VERSION

We could use a macro to avoid code duplication for reading and setting
the X/Y/Z fields in all the load commands. But since there're no users
of the fields except od, we can just add a function that properly prints
all version components out of 4-byte unsigned integer and remove the
version subfields until they're really needed.

This also fixes incorrect length of the first version subfield as it
should be two bytes long instead of one:
  X.Y.Z is encoded in nibbles xxxx.yy.zz
---
 bfd/ChangeLog       |  7 +++++++
 bfd/mach-o.c        |  6 +-----
 bfd/mach-o.h        |  6 ++----
 binutils/ChangeLog  |  5 +++++
 binutils/od-macho.c | 13 ++++++++++++-
 5 files changed, 27 insertions(+), 10 deletions(-)

-- 
2.19.1

Comments

tgingold@free.fr Nov. 6, 2018, 2:25 p.m. | #1
Hello,

I am ok with your whole patchset, but:

> +static void

> +printf_version (uint32_t version) {

> +  uint32_t maj, min, upd;

> +  maj = (version >> 16) & 0xffff;

> +  min = (version >> 8) & 0xff;

> +  upd = version & 0xff;

> +  printf ("%u.%u.%u", maj, min, upd);

> +}


this doesn't follow the GNU code style.  The '{' must be on a new line.

I haven't upgraded to latest Darwin, so I haven't checked the code.

Thanks,
Tristan.

Patch

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index d8b50f5381..4015c26cbf 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@ 
+2018-11-06  Roman Bolshakov <r.bolshakov@yadro.com>
+
+	* mach-o.h (bfd_mach_o_version_min_command): Don't split version into
+	a few fields.
+	* mach-o.c (bfd_mach_o_read_version_min): Don't split version into a
+	few fields.
+
 2018-11-02  Alan Modra  <amodra@gmail.com>
 
 	PR 23850
diff --git a/bfd/mach-o.c b/bfd/mach-o.c
index 1b461f5988..7e4aa7d068 100644
--- a/bfd/mach-o.c
+++ b/bfd/mach-o.c
@@ -4598,15 +4598,11 @@  bfd_mach_o_read_version_min (bfd *abfd, bfd_mach_o_load_command *command)
 {
   bfd_mach_o_version_min_command *cmd = &command->command.version_min;
   struct mach_o_version_min_command_external raw;
-  unsigned int ver;
 
   if (bfd_bread (&raw, sizeof (raw), abfd) != sizeof (raw))
     return FALSE;
 
-  ver = bfd_get_32 (abfd, raw.version);
-  cmd->rel = ver >> 16;
-  cmd->maj = ver >> 8;
-  cmd->min = ver;
+  cmd->version = bfd_get_32 (abfd, raw.version);
   cmd->reserved = bfd_get_32 (abfd, raw.reserved);
   return TRUE;
 }
diff --git a/bfd/mach-o.h b/bfd/mach-o.h
index d80d43991e..4fd229f352 100644
--- a/bfd/mach-o.h
+++ b/bfd/mach-o.h
@@ -519,10 +519,8 @@  bfd_mach_o_dyld_info_command;
 
 typedef struct bfd_mach_o_version_min_command
 {
-  unsigned char rel;
-  unsigned char maj;
-  unsigned char min;
-  unsigned int reserved;
+  uint32_t version;
+  uint32_t reserved;
 }
 bfd_mach_o_version_min_command;
 
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index bdbee0fff6..d8970aae88 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-11-06  Roman Bolshakov  <r.bolshakov@yadro.com>
+
+	* od-macho.c (printf_version): New.
+	(dump_load_command): Use it to print version.
+
 2018-11-03  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* elfedit.c (update_elf_header): Move EI_MAG? check to ...
diff --git a/binutils/od-macho.c b/binutils/od-macho.c
index 8153adae92..98551118c0 100644
--- a/binutils/od-macho.c
+++ b/binutils/od-macho.c
@@ -1438,6 +1438,15 @@  dump_twolevel_hints (bfd *abfd, bfd_mach_o_twolevel_hints_command *cmd)
   free (buf);
 }
 
+static void
+printf_version (uint32_t version) {
+  uint32_t maj, min, upd;
+  maj = (version >> 16) & 0xffff;
+  min = (version >> 8) & 0xff;
+  upd = version & 0xff;
+  printf ("%u.%u.%u", maj, min, upd);
+}
+
 static void
 dump_load_command (bfd *abfd, bfd_mach_o_load_command *cmd,
                    unsigned int idx, bfd_boolean verbose)
@@ -1585,7 +1594,9 @@  dump_load_command (bfd *abfd, bfd_mach_o_load_command *cmd,
       {
         bfd_mach_o_version_min_command *ver = &cmd->command.version_min;
 
-        printf ("    %u.%u.%u\n", ver->rel, ver->maj, ver->min);
+        printf ("   os: ");
+        printf_version (ver->version);
+        printf ("\n");
       }
       break;
     case BFD_MACH_O_LC_SOURCE_VERSION: