bug#16578: Wish: Support for non-native endianness in od

[Pádraig Brady]

On 02/02/2014 01:20 AM, Pádraig Brady wrote:
> On 01/31/2014 09:44 AM, Niels Möller wrote:
>> address@hidden (Niels Möller) writes:
>>
>>> Pádraig Brady <address@hidden> writes:
>>>> I agree this would be useful and easy enough to add.
>>>> I suppose the interface would be --endian=little|big
>>>
>>> Maybe I can have a look at what it takes.
>>
>> Below is a crude patch (missing: usage message, tests cases, docs,
>> translation). I think it should work fine for floats too. I see no
>> obvious and more beautiful way to do it. 
>>
>> (And I think I have copyright assignment papers for coreutils in place,
>> since work on factor some year ago).
>>
>> Regards,
>> /Niels
>>
>> diff --git a/src/od.c b/src/od.c
>> index 514fe50..a71e302 100644
>> --- a/src/od.c
>> +++ b/src/od.c
>> @@ -259,13 +259,16 @@ static enum size_spec 
>> integral_type_size[MAX_INTEGRAL_TYPE_SIZE + 1];
>>  #define MAX_FP_TYPE_SIZE sizeof (long double)
>>  static enum size_spec fp_type_size[MAX_FP_TYPE_SIZE + 1];
>>  
>> +bool input_swap;
>> +
>>  static char const short_options[] = "A:aBbcDdeFfHhIij:LlN:OoS:st:vw::Xx";
>>  
>>  /* For long options that have no equivalent short option, use a
>>     non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
>>  enum
>>  {
>> -  TRADITIONAL_OPTION = CHAR_MAX + 1
>> +  TRADITIONAL_OPTION = CHAR_MAX + 1,
>> +  ENDIAN_OPTION,
>>  };
>>  
>>  static struct option const long_options[] =
>> @@ -278,6 +281,7 @@ static struct option const long_options[] =
>>    {"strings", optional_argument, NULL, 'S'},
>>    {"traditional", no_argument, NULL, TRADITIONAL_OPTION},
>>    {"width", optional_argument, NULL, 'w'},
>> +  {"endian", required_argument, NULL, ENDIAN_OPTION },
>>  
>>    {GETOPT_HELP_OPTION_DECL},
>>    {GETOPT_VERSION_OPTION_DECL},
>> @@ -406,7 +410,21 @@ N (size_t fields, size_t blank, void const *block,      
>>                 \
>>      {                                                                   \
>>        int next_pad = pad * (i - 1) / fields;                            \
>>        int adjusted_width = pad_remaining - next_pad + width;            \
>> -      T x = *p++;                                                       \
>> +      T x;                                                              \
>> +      if (input_swap && sizeof(T) > 1)                                  \
>> +        {                                                               \
>> +          int j;                                                        \
>> +          union {                                                       \
>> +            T x;                                                        \
>> +            char b[sizeof(T)];                                          \
>> +          } u;                                                          \
>> +          for (j = 0; j < sizeof(T); j++)                               \
>> +            u.b[j] = ((const char *) p)[sizeof(T) - 1 - j];             \
>> +          x = u.x;                                                      \
>> +        }                                                               \
>> +      else                                                              \
>> +        x = *p;                                                         \
>> +      p++;                                                              \
>>        ACTION;                                                           \
>>        pad_remaining = next_pad;                                         \
>>      }                                                                   \
>> @@ -1664,6 +1682,24 @@ main (int argc, char **argv)
>>            traditional = true;
>>            break;
>>  
>> +        case ENDIAN_OPTION:
>> +          if (!strcmp (optarg, "big"))
>> +            {
>> +#if !WORDS_BIGENDIAN
>> +              input_swap = true;
>> +#endif
>> +            }
>> +          else if (!strcmp (optarg, "little"))
>> +            {
>> +#if WORDS_BIGENDIAN
>> +                input_swap = true;
>> +#endif
>> +            }
>> +          else
>> +            error (EXIT_FAILURE, 0,
>> +                   _("bad argument '%s' for --endian option"), optarg);
>> +          break;
>> +
>>            /* The next several cases map the traditional format
>>               specification options to the corresponding modern format
>>               specs.  GNU od accepts any combination of old- and
> 
> That looks good.
> I'll adjust slightly to use XARGMATCH and add some docs/tests.
> I'm travelling at the moment but merge this soon.

Attached in the patch I intend to push in your name.

I changed the option handling to reuse the XARGMATCH functionality.
Also I changed things slightly so as the last --endian option
specified wins. Previously we only set the input_swap variable
to true, never to false. On a related point I set the input_swap
global to be static.

I also added docs to usage() and the texinfo file, and added a test.

BTW I checked if there was any speed difference with the new code.
I wasn't expecting this to be a bottleneck, and true enough
there is only a marginal change. The new code is consistently
a little _faster_ though on my i3-2310M which is a bit surprising.

 $ truncate -s1G od.in
 $ time od.old -tx8 od.in
 5.05 elapsed
 $ time od.new -tx8 --endian=bug od.in
 4.97 elapsed

My hunch is there is more pretching happening in the new version,
but can't check on this system due to:

  $ perf stat -e L1-dcache-prefetches:u true
      <not supported> L1-dcache-prefetches:u

For kicks I put in bswap_{16,32,64}() calls which are guaranteed
available by gnulib, but replaced with architecture specific asm
on this system, and the speed regressed back to that of od.old.

thanks,
Pádraig.

od--endian.patch
Description: Text Data