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

Previous Next

Package: coreutils;

Reported by: nisse <at> lysator.liu.se (Niels Möller)

Date: Tue, 28 Jan 2014 13:27:01 UTC

Severity: normal

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: nisse <at> lysator.liu.se (Niels Möller)
Subject: bug#16578: closed (Re: bug#16578: Wish: Support for non-native
 endianness in od)
Date: Sat, 08 Feb 2014 22:02:03 +0000
[Message part 1 (text/plain, inline)]
Your bug report

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

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 16578 <at> debbugs.gnu.org.

-- 
16578: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16578
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Pádraig Brady <P <at> draigBrady.com>
To: Niels Möller <nisse <at> lysator.liu.se>
Cc: 16578-done <at> debbugs.gnu.org
Subject: Re: bug#16578: Wish: Support for non-native endianness in od
Date: Sat, 08 Feb 2014 22:01:36 +0000
[Message part 3 (text/plain, inline)]
On 02/02/2014 01:20 AM, Pádraig Brady wrote:
> On 01/31/2014 09:44 AM, Niels Möller wrote:
>> nisse <at> lysator.liu.se (Niels Möller) writes:
>>
>>> Pádraig Brady <P <at> draigBrady.com> 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 (text/x-patch, attachment)]
[Message part 5 (message/rfc822, inline)]
From: nisse <at> lysator.liu.se (Niels Möller)
To: bug-coreutils <at> gnu.org
Subject: Wish: Support for non-native endianness in od
Date: Tue, 28 Jan 2014 13:54:47 +0100
For the "od" program, it would be nice with a flag to specify the
endianness for all types which are larger than a byte. Possible
alternatives could be "big endian", "little endian", "native endian".

And for floats, besides endianness, it would be nice to be able to
specify native format or ieee format, for systems where these are
different.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.




This bug report was last modified 11 years and 99 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.