Skip to content

Commit

Permalink
STYLE: Clean-up ByteSwapper, using private SwapBytes helper function
Browse files Browse the repository at this point in the history
Aims to reduce code duplication. Inspired by a suggestion from Bradley Lowekamp
at https://github.com/InsightSoftwareConsortium/ITK/pull/4841/files#r1751777344
  • Loading branch information
N-Dekker committed Sep 20, 2024
1 parent 0b03a81 commit 9d241e9
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 69 deletions.
1 change: 1 addition & 0 deletions .github/workflows/itk_dict.txt
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ btw
buf
bugfix
bw
byteswap
byu
bzip
calc
Expand Down
5 changes: 5 additions & 0 deletions Modules/Core/Common/include/itkByteSwapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ class ITK_TEMPLATE_EXPORT ByteSwapper : public Object
SwapWrite8Range(const void * ptr, BufferSizeType num, OStreamType * fp);

private:
/** Swaps the bytes of the specified argument in-place. Assumes that its number of bytes is either 2, 4, or 8.
* Otherwise, it throws an exception. */
static void
SwapBytes(T &);

static constexpr bool m_SystemIsBigEndian{
#ifdef CMAKE_WORDS_BIGENDIAN
true
Expand Down
106 changes: 37 additions & 69 deletions Modules/Core/Common/include/itkByteSwapper.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
#ifndef itkByteSwapper_hxx
#define itkByteSwapper_hxx
#include "itkMakeUniqueForOverwrite.h"
#include <algorithm> // For swap.
#include <algorithm> // For for_each_n and swap.
#include <cstddef> // For byte.
#include <memory>
#include <cstring>

Expand All @@ -44,49 +45,19 @@ template <typename T>
void
ByteSwapper<T>::SwapFromSystemToBigEndian([[maybe_unused]] T * p)
{
if constexpr (!m_SystemIsBigEndian)
if constexpr (!m_SystemIsBigEndian && sizeof(T) > 1)
{
switch (sizeof(T))
{
case 1:
return;
case 2:
Self::Swap2(p);
return;
case 4:
Self::Swap4(p);
return;
case 8:
Self::Swap8(p);
return;
default:
itkGenericExceptionMacro("Cannot swap number of bytes requested");
}
SwapBytes(*p);
}
}

template <typename T>
void
ByteSwapper<T>::SwapRangeFromSystemToBigEndian([[maybe_unused]] T * p, [[maybe_unused]] BufferSizeType num)
{
if constexpr (!m_SystemIsBigEndian)
if constexpr (!m_SystemIsBigEndian && sizeof(T) > 1)
{
switch (sizeof(T))
{
case 1:
return;
case 2:
Self::Swap2Range(p, num);
return;
case 4:
Self::Swap4Range(p, num);
return;
case 8:
Self::Swap8Range(p, num);
return;
default:
itkGenericExceptionMacro("Cannot swap number of bytes requested");
}
std::for_each_n(p, num, [](T & element) { SwapBytes(element); });
}
}

Expand Down Expand Up @@ -127,49 +98,19 @@ template <typename T>
void
ByteSwapper<T>::SwapFromSystemToLittleEndian([[maybe_unused]] T * p)
{
if constexpr (m_SystemIsBigEndian)
if constexpr (m_SystemIsBigEndian && sizeof(T) > 1)
{
switch (sizeof(T))
{
case 1:
return;
case 2:
Self::Swap2(p);
return;
case 4:
Self::Swap4(p);
return;
case 8:
Self::Swap8(p);
return;
default:
itkGenericExceptionMacro("Cannot swap number of bytes requested");
}
SwapBytes(*p);
}
}

template <typename T>
void
ByteSwapper<T>::SwapRangeFromSystemToLittleEndian([[maybe_unused]] T * p, [[maybe_unused]] BufferSizeType num)
{
if constexpr (m_SystemIsBigEndian)
if constexpr (m_SystemIsBigEndian && sizeof(T) > 1)
{
switch (sizeof(T))
{
case 1:
return;
case 2:
Self::Swap2Range(p, num);
return;
case 4:
Self::Swap4Range(p, num);
return;
case 8:
Self::Swap8Range(p, num);
return;
default:
itkGenericExceptionMacro("Cannot swap number of bytes requested");
}
std::for_each_n(p, num, [](T & element) { SwapBytes(element); });
}
}

Expand Down Expand Up @@ -369,6 +310,33 @@ ByteSwapper<T>::SwapWrite8Range(const void * ptr, BufferSizeType num, OStreamTyp
}
}
}


// The following member function is private:

template <typename T>
void
ByteSwapper<T>::SwapBytes(T & value)
{
static constexpr size_t numberOfBytes = sizeof(T);

// Historically (from ITK v1.2.0, March 2003) the following number of bytes are supported:
if constexpr (numberOfBytes == 2 || numberOfBytes == 4 || numberOfBytes == 8)
{
// When the value is an integer, the following code is equivalent to `value = std::byteswap(value)`, in C++26:
auto * const bytes = reinterpret_cast<std::byte *>(&value);

for (size_t i{}; i < (numberOfBytes / 2); ++i)
{
std::swap(bytes[i], bytes[(numberOfBytes - 1) - i]);
}
}
else
{
itkGenericExceptionMacro("Cannot swap number of bytes requested");
}
}

} // end namespace itk

#endif

0 comments on commit 9d241e9

Please sign in to comment.