Skip to content

Commit

Permalink
[num-limits] add kBitsPerByte, BitSizeOf(), and `BytesForBitSize(…
Browse files Browse the repository at this point in the history
…)` (openthread#9618)

This commit removes the use of `CHAR_BIT` in the code and instead
defines a constant `kBitsPerByte` for this in `numeric_limits.hpp`.
`CHAR_BIT` is generally defined as `8` but there are some platforms
were this may not be the case. The way we use `CHAR_BIT` is to
determine number of bits in a byte, so introducing our own constant
`kBitsPerByte` helps improve code readability and safety.

This commit also adds `BitSizeOf()` macro which is similar to `sizeof()`
but returns the number of bits of a given type or variable. Macro
`BytesForBitSize()` is also added which returns the number of bytes
needed to represent a given bit size. This replaces the
`BitVectorBytes()` macro previously defined in `encoding.hpp`.
  • Loading branch information
abtink authored Nov 20, 2023
1 parent 7936f8b commit ce9edaf
Show file tree
Hide file tree
Showing 32 changed files with 104 additions and 77 deletions.
2 changes: 1 addition & 1 deletion include/openthread/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ extern "C" {
* @note This number versions both OpenThread platform and user APIs.
*
*/
#define OPENTHREAD_API_VERSION (373)
#define OPENTHREAD_API_VERSION (374)

/**
* @addtogroup api-instance
Expand Down
1 change: 1 addition & 0 deletions include/openthread/platform/toolchain.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#define OPENTHREAD_PLATFORM_TOOLCHAIN_H_

#include <stdbool.h>
#include <stdint.h>

#ifdef __cplusplus
extern "C" {
Expand Down
7 changes: 4 additions & 3 deletions src/cli/cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
#include <openthread/radio_stats.h>
#endif
#include "common/new.hpp"
#include "common/numeric_limits.hpp"
#include "common/string.hpp"
#include "mac/channel_mask.hpp"

Expand Down Expand Up @@ -1291,7 +1292,7 @@ template <> otError Interpreter::Process<Cmd("channel")>(Arg aArgs[])
if (otChannelMonitorIsEnabled(GetInstancePtr()))
{
uint32_t channelMask = otLinkGetSupportedChannelMask(GetInstancePtr());
uint8_t channelNum = sizeof(channelMask) * CHAR_BIT;
uint8_t channelNum = BitSizeOf(channelMask);

OutputLine("interval: %lu", ToUlong(otChannelMonitorGetSampleInterval(GetInstancePtr())));
OutputLine("threshold: %d", otChannelMonitorGetRssiThreshold(GetInstancePtr()));
Expand Down Expand Up @@ -2611,7 +2612,7 @@ template <> otError Interpreter::Process<Cmd("discover")>(Arg aArgs[])
uint8_t channel;

SuccessOrExit(error = aArgs[0].ParseAsUint8(channel));
VerifyOrExit(channel < sizeof(scanChannels) * CHAR_BIT, error = OT_ERROR_INVALID_ARGS);
VerifyOrExit(channel < BitSizeOf(scanChannels), error = OT_ERROR_INVALID_ARGS);
scanChannels = 1 << channel;
}

Expand Down Expand Up @@ -7113,7 +7114,7 @@ template <> otError Interpreter::Process<Cmd("scan")>(Arg aArgs[])
uint8_t channel;

SuccessOrExit(error = aArgs->ParseAsUint8(channel));
VerifyOrExit(channel < sizeof(scanChannels) * CHAR_BIT, error = OT_ERROR_INVALID_ARGS);
VerifyOrExit(channel < BitSizeOf(scanChannels), error = OT_ERROR_INVALID_ARGS);
scanChannels = 1 << channel;
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/coap/coap_message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ Error Option::Iterator::ReadOptionValue(uint64_t &aUintValue) const

for (uint16_t pos = 0; pos < mOption.mLength; pos++)
{
aUintValue <<= CHAR_BIT;
aUintValue <<= kBitsPerByte;
aUintValue |= buffer[pos];
}

Expand Down
3 changes: 2 additions & 1 deletion src/core/common/bit_vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "common/debug.hpp"
#include "common/encoding.hpp"
#include "common/equatable.hpp"
#include "common/numeric_limits.hpp"

namespace ot {

Expand Down Expand Up @@ -122,7 +123,7 @@ template <uint16_t N> class BitVector : public Equatable<BitVector<N>>, public C
}

private:
uint8_t mMask[BitVectorBytes(N)];
uint8_t mMask[BytesForBitSize(N)];
};

/**
Expand Down
3 changes: 0 additions & 3 deletions src/core/common/encoding.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#endif
#endif

#include <limits.h>
#include <stdint.h>

namespace ot {
Expand Down Expand Up @@ -82,8 +81,6 @@ inline uint32_t Reverse32(uint32_t v)
return v;
}

#define BitVectorBytes(x) static_cast<uint8_t>(((x) + (CHAR_BIT - 1)) / CHAR_BIT)

namespace BigEndian {

#if BYTE_ORDER_BIG_ENDIAN
Expand Down
2 changes: 1 addition & 1 deletion src/core/common/notifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void Notifier::LogEvents(Events aEvents) const
bool didLog = false;
String<kFlagsStringBufferSize> string;

for (uint8_t bit = 0; bit < sizeof(Events::Flags) * CHAR_BIT; bit++)
for (uint8_t bit = 0; bit < BitSizeOf(Events::Flags); bit++)
{
VerifyOrExit(flags != 0);

Expand Down
22 changes: 22 additions & 0 deletions src/core/common/numeric_limits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,28 @@

namespace ot {

static constexpr uint8_t kBitsPerByte = 8; ///< Number of bits in a byte.

/**
* Returns the bit-size (number of bits) of a given type or variable.
*
* @param[in] aItem The item (type or variable or expression) to get the bit-size of.
*
* @returns Number of bits of @p aItem.
*
*/
#define BitSizeOf(aItem) (sizeof(aItem) * kBitsPerByte)

/**
* Determines number of byes to represent a given number of bits.
*
* @param[in] aBitSize The bit-size (number of bits).
*
* @returns Number of bytes to represent @p aBitSize.
*
*/
#define BytesForBitSize(aBitSize) static_cast<uint8_t>(((aBitSize) + (kBitsPerByte - 1)) / kBitsPerByte)

/**
* Provides a way to query properties of arithmetic types.
*
Expand Down
5 changes: 2 additions & 3 deletions src/core/common/time_ticker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@

#include "openthread-core-config.h"

#include <limits.h>

#include "common/locator.hpp"
#include "common/non_copyable.hpp"
#include "common/numeric_limits.hpp"
#include "common/time.hpp"
#include "common/timer.hpp"

Expand Down Expand Up @@ -124,7 +123,7 @@ class TimeTicker : public InstanceLocator, private NonCopyable
uint32_t mReceivers;
TickerTimer mTimer;

static_assert(kNumReceivers < sizeof(mReceivers) * CHAR_BIT, "Too many `Receiver`s - does not fit in a bit mask");
static_assert(kNumReceivers < BitSizeOf(mReceivers), "Too many `Receiver`s - does not fit in a bit mask");
};

} // namespace ot
Expand Down
2 changes: 0 additions & 2 deletions src/core/crypto/aes_ccm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@

#include "aes_ccm.hpp"

#include <limits.h>

#include "common/code_utils.hpp"
#include "common/debug.hpp"
#include "common/encoding.hpp"
Expand Down
2 changes: 1 addition & 1 deletion src/core/crypto/crypto_platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ OT_TOOL_WEAK otError otPlatCryptoAesSetKey(otCryptoContext *aContext, const otCr
VerifyOrExit(aContext->mContextSize >= sizeof(mbedtls_aes_context), error = kErrorFailed);

context = static_cast<mbedtls_aes_context *>(aContext->mContext);
VerifyOrExit((mbedtls_aes_setkey_enc(context, key.GetBytes(), (key.GetLength() * CHAR_BIT)) == 0),
VerifyOrExit((mbedtls_aes_setkey_enc(context, key.GetBytes(), (key.GetLength() * kBitsPerByte)) == 0),
error = kErrorFailed);

exit:
Expand Down
8 changes: 4 additions & 4 deletions src/core/mac/channel_mask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@

#include "openthread-core-config.h"

#include <limits.h>
#include <openthread/platform/radio.h>

#include "common/equatable.hpp"
#include "common/numeric_limits.hpp"
#include "common/string.hpp"
#include "radio/radio.hpp"

Expand Down Expand Up @@ -147,7 +147,7 @@ class ChannelMask : public Unequatable<ChannelMask>
*/
bool ContainsChannel(uint8_t aChannel) const
{
return (aChannel < sizeof(mMask) * CHAR_BIT) ? ((1UL << aChannel) & mMask) != 0 : false;
return (aChannel < BitSizeOf(mMask)) ? ((1UL << aChannel) & mMask) != 0 : false;
}

/**
Expand All @@ -158,7 +158,7 @@ class ChannelMask : public Unequatable<ChannelMask>
*/
void AddChannel(uint8_t aChannel)
{
if (aChannel < sizeof(mMask) * CHAR_BIT)
if (aChannel < BitSizeOf(mMask))
{
mMask |= (1UL << aChannel);
}
Expand All @@ -172,7 +172,7 @@ class ChannelMask : public Unequatable<ChannelMask>
*/
void RemoveChannel(uint8_t aChannel)
{
if (aChannel < sizeof(mMask) * CHAR_BIT)
if (aChannel < BitSizeOf(mMask))
{
mMask &= ~(1UL << aChannel);
}
Expand Down
10 changes: 4 additions & 6 deletions src/core/mac/mac_frame.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,10 @@

#include "openthread-core-config.h"

#include <limits.h>
#include <stdint.h>

#include "common/as_core_type.hpp"
#include "common/const_cast.hpp"
#include "common/encoding.hpp"
#include "common/numeric_limits.hpp"
#include "mac/mac_types.hpp"
#include "meshcop/network_name.hpp"

Expand Down Expand Up @@ -1115,9 +1113,9 @@ class Frame : public otRadioFrame
static constexpr uint8_t kKeyIdModeMask = 3 << 3;

static constexpr uint8_t kMic0Size = 0;
static constexpr uint8_t kMic32Size = 32 / CHAR_BIT;
static constexpr uint8_t kMic64Size = 64 / CHAR_BIT;
static constexpr uint8_t kMic128Size = 128 / CHAR_BIT;
static constexpr uint8_t kMic32Size = 32 / kBitsPerByte;
static constexpr uint8_t kMic64Size = 64 / kBitsPerByte;
static constexpr uint8_t kMic128Size = 128 / kBitsPerByte;
static constexpr uint8_t kMaxMicSize = kMic128Size;

static constexpr uint8_t kKeySourceSizeMode0 = 0;
Expand Down
10 changes: 5 additions & 5 deletions src/core/meshcop/meshcop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ void JoinerDiscerner::CopyTo(Mac::ExtAddress &aExtAddress) const
OT_ASSERT(IsValid());

// Write full bytes
while (remaining >= CHAR_BIT)
while (remaining >= kBitsPerByte)
{
*cur = static_cast<uint8_t>(value & 0xff);
value >>= CHAR_BIT;
value >>= kBitsPerByte;
cur--;
remaining -= CHAR_BIT;
remaining -= kBitsPerByte;
}

// Write any remaining bits (not a full byte)
Expand All @@ -168,11 +168,11 @@ JoinerDiscerner::InfoString JoinerDiscerner::ToString(void) const
{
InfoString string;

if (mLength <= sizeof(uint16_t) * CHAR_BIT)
if (mLength <= BitSizeOf(uint16_t))
{
string.Append("0x%04x", static_cast<uint16_t>(mValue));
}
else if (mLength <= sizeof(uint32_t) * CHAR_BIT)
else if (mLength <= BitSizeOf(uint32_t))
{
string.Append("0x%08lx", ToUlong(static_cast<uint32_t>(mValue)));
}
Expand Down
9 changes: 4 additions & 5 deletions src/core/meshcop/meshcop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@

#include "openthread-core-config.h"

#include <limits.h>

#include <openthread/commissioner.h>
#include <openthread/instance.h>
#include <openthread/joiner.h>
Expand All @@ -49,6 +47,7 @@
#include "common/equatable.hpp"
#include "common/log.hpp"
#include "common/message.hpp"
#include "common/numeric_limits.hpp"
#include "common/string.hpp"
#include "mac/mac_types.hpp"
#include "meshcop/meshcop_tlvs.hpp"
Expand Down Expand Up @@ -393,10 +392,10 @@ class SteeringData : public otSteeringData
private:
static constexpr uint8_t kPermitAll = 0xff;

uint8_t GetNumBits(void) const { return (mLength * CHAR_BIT); }
uint8_t GetNumBits(void) const { return (mLength * kBitsPerByte); }

uint8_t BitIndex(uint8_t aBit) const { return (mLength - 1 - (aBit / CHAR_BIT)); }
uint8_t BitFlag(uint8_t aBit) const { return static_cast<uint8_t>(1U << (aBit % CHAR_BIT)); }
uint8_t BitIndex(uint8_t aBit) const { return (mLength - 1 - (aBit / kBitsPerByte)); }
uint8_t BitFlag(uint8_t aBit) const { return static_cast<uint8_t>(1U << (aBit % kBitsPerByte)); }

bool GetBit(uint8_t aBit) const { return (m8[BitIndex(aBit)] & BitFlag(aBit)) != 0; }
void SetBit(uint8_t aBit) { m8[BitIndex(aBit)] |= BitFlag(aBit); }
Expand Down
3 changes: 2 additions & 1 deletion src/core/meshcop/meshcop_tlvs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "common/const_cast.hpp"
#include "common/debug.hpp"
#include "common/num_utils.hpp"
#include "common/numeric_limits.hpp"
#include "common/string.hpp"
#include "meshcop/meshcop.hpp"

Expand Down Expand Up @@ -146,7 +147,7 @@ bool ChannelTlv::IsValid(void) const
bool ret = false;

VerifyOrExit(GetLength() == sizeof(*this) - sizeof(Tlv));
VerifyOrExit(mChannelPage < sizeof(uint32_t) * CHAR_BIT);
VerifyOrExit(mChannelPage < BitSizeOf(uint32_t));
VerifyOrExit((1U << mChannelPage) & Radio::kSupportedChannelPages);
VerifyOrExit(Radio::kChannelMin <= GetChannel() && GetChannel() <= Radio::kChannelMax);
ret = true;
Expand Down
6 changes: 4 additions & 2 deletions src/core/net/ip4_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
*/

#include "ip4_types.hpp"
#include "ip6_address.hpp"

#include "common/numeric_limits.hpp"
#include "net/ip6_address.hpp"

namespace ot {
namespace Ip4 {
Expand Down Expand Up @@ -92,7 +94,7 @@ void Address::ExtractFromIp6Address(uint8_t aPrefixLength, const Ip6::Address &a

OT_ASSERT(Ip6::Prefix::IsValidNat64PrefixLength(aPrefixLength));

ip6Index = aPrefixLength / CHAR_BIT;
ip6Index = aPrefixLength / kBitsPerByte;

for (uint8_t &i : mFields.m8)
{
Expand Down
10 changes: 5 additions & 5 deletions src/core/net/ip6_address.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ bool Prefix::operator<(const Prefix &aOther) const
ExitNow();
}

isSmaller = GetBytes()[matchedLength / CHAR_BIT] < aOther.GetBytes()[matchedLength / CHAR_BIT];
isSmaller = GetBytes()[matchedLength / kBitsPerByte] < aOther.GetBytes()[matchedLength / kBitsPerByte];

exit:
return isSmaller;
Expand All @@ -150,7 +150,7 @@ uint8_t Prefix::MatchLength(const uint8_t *aPrefixA, const uint8_t *aPrefixB, ui

if (diff == 0)
{
matchedLength += CHAR_BIT;
matchedLength += kBitsPerByte;
}
else
{
Expand Down Expand Up @@ -405,8 +405,8 @@ void Address::CopyBits(uint8_t *aDst, const uint8_t *aSrc, uint8_t aNumBits)
// the case where `aNumBits` may not be a multiple of 8. It leaves the
// remaining bits beyond `aNumBits` in `aDst` unchanged.

uint8_t numBytes = aNumBits / CHAR_BIT;
uint8_t extraBits = aNumBits % CHAR_BIT;
uint8_t numBytes = aNumBits / kBitsPerByte;
uint8_t extraBits = aNumBits % kBitsPerByte;

memcpy(aDst, aSrc, numBytes);

Expand Down Expand Up @@ -520,7 +520,7 @@ void Address::SynthesizeFromIp4Address(const Prefix &aPrefix, const Ip4::Address
Clear();
SetPrefix(aPrefix);

ip6Index = aPrefix.GetLength() / CHAR_BIT;
ip6Index = aPrefix.GetLength() / kBitsPerByte;

for (uint8_t i = 0; i < Ip4::Address::kSize; i++)
{
Expand Down
Loading

0 comments on commit ce9edaf

Please sign in to comment.