Skip to content

include: util: Add generic function to count bits set in a value #87943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/releases/release-notes-4.2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ New APIs and options
* :c:func:`sys_clock_gettime`
* :c:func:`sys_clock_settime`
* :c:func:`sys_clock_nanosleep`
* :c:func:`zephyr_count_bits`

* LoRaWAN
* :c:func:`lorawan_request_link_check`
Expand Down
18 changes: 4 additions & 14 deletions drivers/adc/adc_sam.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <zephyr/logging/log.h>
#include <zephyr/irq.h>
#include <zephyr/sys/util.h>
LOG_MODULE_REGISTER(adc_sam, CONFIG_ADC_LOG_LEVEL);

#define SAM_ADC_NUM_CHANNELS 16
Expand Down Expand Up @@ -51,18 +52,6 @@ struct adc_sam_data {
uint8_t num_active_channels;
};

static uint8_t count_bits(uint32_t val)
{
uint8_t res = 0;

while (val) {
res += val & 1U;
val >>= 1;
}

return res;
}

static int adc_sam_channel_setup(const struct device *dev,
const struct adc_channel_cfg *channel_cfg)
{
Expand Down Expand Up @@ -156,7 +145,8 @@ static void adc_context_start_sampling(struct adc_context *ctx)
const struct adc_sam_config *const cfg = data->dev->config;
Adc *const adc = cfg->regs;

data->num_active_channels = count_bits(ctx->sequence.channels);
data->num_active_channels =
zephyr_count_bits(&ctx->sequence.channels, sizeof(ctx->sequence.channels));

/* Disable all */
adc->ADC_CHDR = 0xffff;
Expand Down Expand Up @@ -222,7 +212,7 @@ static int start_read(const struct device *dev,
return -EINVAL;
}

data->num_active_channels = count_bits(channels);
data->num_active_channels = zephyr_count_bits(&channels, sizeof(channels));

error = check_buffer_size(sequence, data->num_active_channels);
if (error) {
Expand Down
111 changes: 69 additions & 42 deletions include/zephyr/sys/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ extern "C" {
*/

/** @brief Cast @p x, a pointer, to an unsigned integer. */
#define POINTER_TO_UINT(x) ((uintptr_t) (x))
#define POINTER_TO_UINT(x) ((uintptr_t)(x))
/** @brief Cast @p x, an unsigned integer, to a <tt>void*</tt>. */
#define UINT_TO_POINTER(x) ((void *) (uintptr_t) (x))
#define UINT_TO_POINTER(x) ((void *)(uintptr_t)(x))
/** @brief Cast @p x, a pointer, to a signed integer. */
#define POINTER_TO_INT(x) ((intptr_t) (x))
#define POINTER_TO_INT(x) ((intptr_t)(x))
/** @brief Cast @p x, a signed integer, to a <tt>void*</tt>. */
#define INT_TO_POINTER(x) ((void *) (intptr_t) (x))
#define INT_TO_POINTER(x) ((void *)(intptr_t)(x))

#if !(defined(__CHAR_BIT__) && defined(__SIZEOF_LONG__) && defined(__SIZEOF_LONG_LONG__))
# error Missing required predefined macros for BITS_PER_LONG calculation
#error Missing required predefined macros for BITS_PER_LONG calculation
#endif

/** Number of bits in a byte. */
Expand All @@ -68,27 +68,25 @@ extern "C" {
#define NIBBLES_PER_BYTE (BITS_PER_BYTE / BITS_PER_NIBBLE)

/** Number of bits in a long int. */
#define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__)
#define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__)

/** Number of bits in a long long int. */
#define BITS_PER_LONG_LONG (__CHAR_BIT__ * __SIZEOF_LONG_LONG__)
#define BITS_PER_LONG_LONG (__CHAR_BIT__ * __SIZEOF_LONG_LONG__)

/**
* @brief Create a contiguous bitmask starting at bit position @p l
* and ending at position @p h.
*/
#define GENMASK(h, l) \
(((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
#define GENMASK(h, l) (((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

/**
* @brief Create a contiguous 64-bit bitmask starting at bit position @p l
* and ending at position @p h.
*/
#define GENMASK64(h, l) \
(((~0ULL) - (1ULL << (l)) + 1) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
#define GENMASK64(h, l) (((~0ULL) - (1ULL << (l)) + 1) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

/** @brief 0 if @p cond is true-ish; causes a compile error otherwise. */
#define ZERO_OR_COMPILE_ERROR(cond) ((int) sizeof(char[1 - 2 * !(cond)]) - 1)
#define ZERO_OR_COMPILE_ERROR(cond) ((int)sizeof(char[1 - 2 * !(cond)]) - 1)

#if defined(__cplusplus)

Expand All @@ -104,10 +102,9 @@ extern "C" {
*
* This macro is available only from C, not C++.
*/
#define IS_ARRAY(array) \
ZERO_OR_COMPILE_ERROR( \
!__builtin_types_compatible_p(__typeof__(array), \
__typeof__(&(array)[0])))
#define IS_ARRAY(array) \
ZERO_OR_COMPILE_ERROR( \
!__builtin_types_compatible_p(__typeof__(array), __typeof__(&(array)[0])))

/**
* @brief Number of elements in the given @p array
Expand All @@ -118,8 +115,7 @@ extern "C" {
*
* In C, passing a pointer as @p array causes a compile error.
*/
#define ARRAY_SIZE(array) \
((size_t) (IS_ARRAY(array) + (sizeof(array) / sizeof((array)[0]))))
#define ARRAY_SIZE(array) ((size_t)(IS_ARRAY(array) + (sizeof(array) / sizeof((array)[0]))))

#endif /* __cplusplus */

Expand All @@ -140,10 +136,11 @@ extern "C" {
* It is specially useful for cases where flexible arrays are
* used in unions or are not the last element in the struct.
*/
#define FLEXIBLE_ARRAY_DECLARE(type, name) \
struct { \
struct { } __unused_##name; \
type name[]; \
#define FLEXIBLE_ARRAY_DECLARE(type, name) \
struct { \
struct { \
} __unused_##name; \
type name[]; \
}

/**
Expand All @@ -161,7 +158,7 @@ extern "C" {
* @return 1 if @p ptr is part of @p array, 0 otherwise
*/
#define IS_ARRAY_ELEMENT(array, ptr) \
((ptr) && POINTER_TO_UINT(array) <= POINTER_TO_UINT(ptr) && \
((ptr) && POINTER_TO_UINT(array) <= POINTER_TO_UINT(ptr) && \
POINTER_TO_UINT(ptr) < POINTER_TO_UINT(&(array)[ARRAY_SIZE(array)]) && \
(POINTER_TO_UINT(ptr) - POINTER_TO_UINT(array)) % sizeof((array)[0]) == 0)

Expand Down Expand Up @@ -253,9 +250,8 @@ extern "C" {
* @brief Validate CONTAINER_OF parameters, only applies to C mode.
*/
#ifndef __cplusplus
#define CONTAINER_OF_VALIDATE(ptr, type, field) \
BUILD_ASSERT(SAME_TYPE(*(ptr), ((type *)0)->field) || \
SAME_TYPE(*(ptr), void), \
#define CONTAINER_OF_VALIDATE(ptr, type, field) \
BUILD_ASSERT(SAME_TYPE(*(ptr), ((type *)0)->field) || SAME_TYPE(*(ptr), void), \
"pointer type mismatch in CONTAINER_OF");
#else
#define CONTAINER_OF_VALIDATE(ptr, type, field)
Expand All @@ -282,10 +278,10 @@ extern "C" {
* @param field the name of the field within the struct @p ptr points to
* @return a pointer to the structure that contains @p ptr
*/
#define CONTAINER_OF(ptr, type, field) \
({ \
CONTAINER_OF_VALIDATE(ptr, type, field) \
((type *)(((char *)(ptr)) - offsetof(type, field))); \
#define CONTAINER_OF(ptr, type, field) \
({ \
CONTAINER_OF_VALIDATE(ptr, type, field) \
((type *)(((char *)(ptr)) - offsetof(type, field))); \
})

/**
Expand All @@ -309,8 +305,7 @@ extern "C" {
*
* @return Concatenated token.
*/
#define CONCAT(...) \
UTIL_CAT(_CONCAT_, NUM_VA_ARGS_LESS_1(__VA_ARGS__))(__VA_ARGS__)
#define CONCAT(...) UTIL_CAT(_CONCAT_, NUM_VA_ARGS_LESS_1(__VA_ARGS__))(__VA_ARGS__)

/**
* @brief Check if @p ptr is aligned to @p align alignment
Expand All @@ -320,14 +315,14 @@ extern "C" {
/**
* @brief Value of @p x rounded up to the next multiple of @p align.
*/
#define ROUND_UP(x, align) \
((((unsigned long)(x) + ((unsigned long)(align) - 1)) / \
(unsigned long)(align)) * (unsigned long)(align))
#define ROUND_UP(x, align) \
((((unsigned long)(x) + ((unsigned long)(align) - 1)) / (unsigned long)(align)) * \
(unsigned long)(align))

/**
* @brief Value of @p x rounded down to the previous multiple of @p align.
*/
#define ROUND_DOWN(x, align) \
#define ROUND_DOWN(x, align) \
(((unsigned long)(x) / (unsigned long)(align)) * (unsigned long)(align))

/** @brief Value of @p x rounded up to the next word boundary. */
Expand Down Expand Up @@ -690,7 +685,7 @@ char *utf8_lcpy(char *dst, const char *src, size_t n);

#define __z_log2d(x) (32 - __builtin_clz(x) - 1)
#define __z_log2q(x) (64 - __builtin_clzll(x) - 1)
#define __z_log2(x) (sizeof(__typeof__(x)) > 4 ? __z_log2q(x) : __z_log2d(x))
#define __z_log2(x) (sizeof(__typeof__(x)) > 4 ? __z_log2q(x) : __z_log2d(x))

/**
* @brief Compute log2(x)
Expand All @@ -714,7 +709,7 @@ char *utf8_lcpy(char *dst, const char *src, size_t n);
*
* @return ceil(log2(x)) when 1 <= x <= max(type(x)), 0 when x < 1
*/
#define LOG2CEIL(x) ((x) <= 1 ? 0 : __z_log2((x)-1) + 1)
#define LOG2CEIL(x) ((x) <= 1 ? 0 : __z_log2((x) - 1) + 1)

/**
* @brief Compute next highest power of two
Expand All @@ -728,7 +723,7 @@ char *utf8_lcpy(char *dst, const char *src, size_t n);
*
* @return 2^ceil(log2(x)) or 0 if 2^ceil(log2(x)) would saturate 64-bits
*/
#define NHPOT(x) ((x) < 1 ? 1 : ((x) > (1ULL<<63) ? 0 : 1ULL << LOG2CEIL(x)))
#define NHPOT(x) ((x) < 1 ? 1 : ((x) > (1ULL << 63) ? 0 : 1ULL << LOG2CEIL(x)))

/**
* @brief Determine if a buffer exceeds highest address
Expand All @@ -742,9 +737,8 @@ char *utf8_lcpy(char *dst, const char *src, size_t n);
*
* @return true if pointer overflow detected, false otherwise
*/
#define Z_DETECT_POINTER_OVERFLOW(addr, buflen) \
(((buflen) != 0) && \
((UINTPTR_MAX - (uintptr_t)(addr)) <= ((uintptr_t)((buflen) - 1))))
#define Z_DETECT_POINTER_OVERFLOW(addr, buflen) \
(((buflen) != 0) && ((UINTPTR_MAX - (uintptr_t)(addr)) <= ((uintptr_t)((buflen) - 1))))

/**
* @brief XOR n bytes
Expand Down Expand Up @@ -819,6 +813,39 @@ static inline bool util_eq(const void *m1, size_t len1, const void *m2, size_t l
return len1 == len2 && (m1 == m2 || util_memeq(m1, m2, len1));
}

/**
* @brief Returns the number of bits set in a value
*
* @param value The value to count number of bits set of
* @param len The number of octets in @p value
*/
static inline size_t zephyr_count_bits(const void *value, size_t len)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tough, this could be just sys_count_bits() use the well-known sys_ prefix and be aligned with existing utils, for unknown reasons some people prefer to use the zephyr_ prefix. But zephyr_ is not really better, because it is also used by many other software projects, and it does not minimize the likelihood of collisions. Instead, we should use UUID version 5 as a prefix, like 7f756fa7_4e84_529e_b406_276069b9bcd0_.

Suggested change
static inline size_t zephyr_count_bits(const void *value, size_t len)
static inline size_t 7f756fa7_4e84_529e_b406_276069b9bcd0_count_bits(const void *const value, const size_t len)
{

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get your point, and I don't really have a strong preference myself. I went with zephyr_ as that seemed like the least controversial output from #64627

but zephyr_ is not really better, because it is also used by many other software projects

Can you share some of these projects so we can consider them properly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, we should use UUID version 5 as a prefix, like 7f756fa7_4e84_529e_b406_276069b9bcd0_.

The C language prohibits starting identifiers with a number 🫣

Copy link
Contributor

@jfischer-no jfischer-no Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use 30a23962-5d6b-4c76-87aa-cc799d50a6a7 and with "Zephyr RTOS" as namespace, which results in c496e475_323e_5ca0_8fa1_39f91557c0e0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share some of these projects so we can consider them properly?

There are some cloud and cryptobro projects. Just search for zephyr.

{
size_t cnt = 0U;
size_t i = 0U;

#ifdef POPCOUNT
for (; i < len / sizeof(unsigned int); i++) {
unsigned int val = ((const unsigned int *)value)[i];

cnt += POPCOUNT(val);
}
i *= sizeof(unsigned int); /* convert to a uint8_t index for the remainder (if any) */
#endif

for (; i < len; i++) {
uint8_t value_u8 = ((const uint8_t *)value)[i];

/* Implements Brian Kernighan’s Algorithm to count bits */
while (value_u8) {
value_u8 &= (value_u8 - 1);
cnt++;
}
}

return cnt;
}

#ifdef __cplusplus
}
#endif
Expand Down
5 changes: 3 additions & 2 deletions samples/bluetooth/cap_acceptor/src/cap_acceptor_broadcast.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @file
* @brief Bluetooth Common Audio Profile (CAP) Acceptor broadcast.
*
* Copyright (c) 2024 Nordic Semiconductor ASA
* Copyright (c) 2024-2025 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -459,7 +459,8 @@ static int bis_sync_req_cb(struct bt_conn *conn,

LOG_INF("BIS sync request received for %p: 0x%08x", recv_state, bis_sync_req[0]);

if (new_bis_sync_req != BT_BAP_BIS_SYNC_NO_PREF && POPCOUNT(new_bis_sync_req) > 1U) {
if (new_bis_sync_req != BT_BAP_BIS_SYNC_NO_PREF &&
zephyr_count_bits(&new_bis_sync_req, sizeof(new_bis_sync_req)) > 1U) {
LOG_WRN("Rejecting BIS sync request for 0x%08X as we do not support that",
new_bis_sync_req);

Expand Down
15 changes: 3 additions & 12 deletions subsys/bluetooth/audio/audio.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

/*
* Copyright (c) 2022 Codecoup
* Copyright (c) 2025 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -23,6 +24,7 @@
#include <zephyr/bluetooth/hci_types.h>
#include <zephyr/logging/log.h>
#include <zephyr/sys/check.h>
#include <zephyr/sys/util.h>
#include <zephyr/toolchain.h>

#include "audio_internal.h"
Expand Down Expand Up @@ -146,18 +148,7 @@ uint8_t bt_audio_get_chan_count(enum bt_audio_location chan_allocation)
return 1;
}

#ifdef POPCOUNT
return POPCOUNT(chan_allocation);
#else
uint8_t cnt = 0U;

while (chan_allocation != 0U) {
cnt += chan_allocation & 1U;
chan_allocation >>= 1U;
}

return cnt;
#endif
return zephyr_count_bits(&chan_allocation, sizeof(chan_allocation));
}

static bool valid_ltv_cb(struct bt_data *data, void *user_data)
Expand Down
18 changes: 1 addition & 17 deletions subsys/bluetooth/audio/bap_broadcast_sink.c
Original file line number Diff line number Diff line change
Expand Up @@ -1093,22 +1093,6 @@ int bt_bap_broadcast_sink_create(struct bt_le_per_adv_sync *pa_sync, uint32_t br
return 0;
}

static uint8_t bit_count(uint32_t bitfield)
{
#ifdef POPCOUNT
return POPCOUNT(bitfield);
#else
uint8_t cnt = 0U;

while (bitfield != 0U) {
cnt += bitfield & 1U;
bitfield >>= 1U;
}

return cnt;
#endif
}

struct sync_base_info_data {
struct bt_audio_codec_cfg codec_cfgs[CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT];
struct bt_audio_codec_cfg *subgroup_codec_cfg;
Expand Down Expand Up @@ -1293,7 +1277,7 @@ int bt_bap_broadcast_sink_sync(struct bt_bap_broadcast_sink *sink, uint32_t inde
}

/* Validate that number of bits set is within supported range */
bis_count = bit_count(indexes_bitfield);
bis_count = zephyr_count_bits(&indexes_bitfield, sizeof(indexes_bitfield));
if (bis_count > CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT) {
LOG_DBG("Cannot sync to more than %d streams (%u was requested)",
CONFIG_BT_BAP_BROADCAST_SNK_STREAM_COUNT, bis_count);
Expand Down
2 changes: 1 addition & 1 deletion tests/bsim/bluetooth/audio/src/bap_broadcast_sink_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ static void test_broadcast_sync(const uint8_t broadcast_code[BT_ISO_BROADCAST_CO
return;
}

stream_sync_cnt = POPCOUNT(bis_index_bitfield);
stream_sync_cnt = zephyr_count_bits(&bis_index_bitfield, sizeof(bis_index_bitfield));
}

static void test_broadcast_sync_inval(void)
Expand Down
Loading