Right Endian conversion
in [C++]
|
From: Giulio Guarnone on 5 Aug 2010 05:00 Hi to all, I've written down this function for perfoming endian conversion : template<typename T> T swap_endian(const T& source) { T ret = 0; for (int i = 0; i < sizeof(T); ++i) { *(reinterpret_cast<char*>(&ret) + i) = *(reinterpret_cast<char*>(&source) + sizeof(T) - i - 1); } return ret; } It seems to work the most of the times, but I'm not sure when I use it on signed types (int long etc etc) with negative numbers or with positive numbers like 1520 : bigendian 0x05F0 littleendian 0xF005 Is it correct, or has anyone a better routine ? Thanx Giulio -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: red floyd on 5 Aug 2010 12:36 On Aug 5, 1:00 pm, Giulio Guarnone <giulio.guarn...(a)gmail.com> wrote: > Hi to all, > > I've written down this function for perfoming endian conversion : > > template<typename T> > T swap_endian(const T& source) > { > T ret = 0; > > for (int i = 0; i < sizeof(T); ++i) > { > *(reinterpret_cast<char*>(&ret) + i) = > *(reinterpret_cast<char*>(&source) + sizeof(T) - i - 1); > } > return ret; > > } > > It seems to work the most of the times, but I'm not sure when I use it > on signed types (int long etc etc) with negative numbers or with > positive numbers like 1520 : > > bigendian 0x05F0 > littleendian 0xF005 > > Is it correct, or has anyone a better routine ? > It looks to me like you're swapping each byte *twice*. I think your loop should only go to sizeof(T)/2 -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Ulrich Eckhardt on 5 Aug 2010 19:25 Giulio Guarnone wrote: > I've written down this function for perfoming endian conversion : > > template<typename T> > T swap_endian(const T& source) > { > T ret = 0; > > for (int i = 0; i < sizeof(T); ++i) > { > *(reinterpret_cast<char*>(&ret) + i) = > *(reinterpret_cast<char*>(&source) + sizeof(T) - i - 1); > } > return ret; > } > > It seems to work the most of the times, but I'm not sure when I use it > on signed types (int long etc etc) with negative numbers or with > positive numbers [...] There is one problem, and that is that byte-swapping might yield an invalid value for a T. Also, logically, the result is not a T but a byte-buffer containing the byte-swapped represention for a T. Of course, other problems also arise when you try to use this function with anything but very simple datatypes. Other than that, the code is correct. Since you only take bytes (char) and move them around, the type of the underlying object is irrelevant. That said, the code is horrible to read. I'd introduce two local objects containing the source and target addresses and then use x[i] for indexing instead of *(x+i). Further, you could make it a bit more generic by initialising 'ret' with 'T()' instead of '0'. However, you don't need to initialise it at all, since it is completely overwritten anyway. BTW, in reply to red floyd's, he's wrong. I guess he overlooked that you are not byte-swapping in place but rather copying. Maybe he got confused be the function name, I got confused by it, too. Uli -- Sator Laser GmbH Gesch�ftsf�hrer: Thorsten F�cking, Amtsgericht Hamburg HR B62 932 [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Stefan van Kessel on 5 Aug 2010 19:21 On 8/6/2010 5:36 AM, red floyd wrote: > On Aug 5, 1:00 pm, Giulio Guarnone<giulio.guarn...(a)gmail.com> wrote: >> template<typename T> >> T swap_endian(const T& source) >> { >> T ret = 0; >> >> for (int i = 0; i< sizeof(T); ++i) >> { >> *(reinterpret_cast<char*>(&ret) + i) = >> *(reinterpret_cast<char*>(&source) + sizeof(T) - i - 1); >> } >> return ret; >> >> } > It looks to me like you're swapping each byte *twice*. > > I think your loop should only go to sizeof(T)/2 The code doesn't actually swap but just sets T byte for byte from back of source to front of source, leaving source itself intact. So sizeof(T) is correct. Just one minor mistake: *(reinterpret_cast<char*>(&source) should be *(reinterpret_cast<const char*>(&source). &source is pointer to const and the compiler shouldn't allow a reinterpret_cast to a pointer to non const. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Jens Schmidt on 5 Aug 2010 19:26
red floyd wrote: > On Aug 5, 1:00 pm, Giulio Guarnone <giulio.guarn...(a)gmail.com> wrote: >> template<typename T> >> T swap_endian(const T& source) >> { >> T ret = 0; >> >> for (int i = 0; i < sizeof(T); ++i) >> { >> *(reinterpret_cast<char*>(&ret) + i) = >> *(reinterpret_cast<char*>(&source) + sizeof(T) - i - 1); >> } >> return ret; >> >> } > > It looks to me like you're swapping each byte *twice*. No, he is not. He is swapping while copying from source to ret. > I think your loop should only go to sizeof(T)/2 That is for in-place swapping only. You can't do in-place swapping with a const T&. A real problem I see: after swapping the value is no longer of type T, but just a sequence of bytes. This might be fatal for the program if T is float or double and accidentially the result is the pattern of a signaling NaN. -- Greetings, Jens Schmidt [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |