Skip to content
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

include: util: Add memeq and bool_memcmp #84159

Open
wants to merge 3 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
35 changes: 35 additions & 0 deletions include/zephyr/sys/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <zephyr/types.h>
#include <stddef.h>
#include <stdint.h>
#include <string.h>

/** @brief Number of bits that make up a type */
#define NUM_BITS(t) (sizeof(t) * BITS_PER_BYTE)
Expand Down Expand Up @@ -783,6 +784,40 @@ static inline void mem_xor_128(uint8_t dst[16], const uint8_t src1[16], const ui
mem_xor_n(dst, src1, src2, 16);
}

/**
* @brief Compare memory areas. The same way as `memcmp` it assume areas to be
* the same length
*
* @param m1 First memory area to compare, cannot be NULL even if length is 0
* @param m2 Second memory area to compare, cannot be NULL even if length is 0
* @param n First n bytes of @p m1 and @p m2 to compares
*
* @returns true if the @p n first bytes of @p m1 and @p m2 are the same, else
* false
*/
static inline bool z_util_memeq(const void *m1, const void *m2, size_t n)
Copy link
Member

Choose a reason for hiding this comment

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

z_ shall not be used for any public APIs, it is a intended to be used for internal/private APIs only.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Is that documented anywhere?

never made it in the docs, https://github.com/zephyrproject-rtos/zephyr/pull/62198/files. z_ was added long time ago to replace _ usage in the tree, but unfortuantly it was seen by many as the zephyr designated prefix for APIs and we ended up with public APIs using z_....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something that must be documented. It seems that there is quite a bunch of public APIs now using the z_, is there good reasons to continue to have that hidden rule?

To me it seems that without proper documentation some more will definitely be added and then I don't see the point of blocking it here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nashif do you have alternative suggestions for naming?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just util_memeq?

{
return memcmp(m1, m2, n) == 0;
}

/**
* @brief Compare memory areas and their length
*
* If the length are 0, return true.
*
* @param m1 First memory area to compare, cannot be NULL even if length is 0
* @param len1 Length of the first memory area to compare
* @param m2 Second memory area to compare, cannot be NULL even if length is 0
* @param len2 Length of the second memory area to compare
*
* @returns true if both the length of the memory areas and their content are
* equal else false
*/
static inline bool z_util_eq(const void *m1, size_t len1, const void *m2, size_t len2)
{
return len1 == len2 && z_util_memeq(m1, m2, len1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I recommend shorting the call to z_util_memeq with m1 == m2 || z_util_memeq(m1, m2, len)

}

#ifdef __cplusplus
}
#endif
Expand Down
3 changes: 2 additions & 1 deletion subsys/bluetooth/host/keys.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef ZEPHYR_SUBSYS_BLUETOOTH_HOST_KEYS_H_
#define ZEPHYR_SUBSYS_BLUETOOTH_HOST_KEYS_H_

#include <zephyr/sys/util.h>
#include <zephyr/bluetooth/bluetooth.h>

/** @cond INTERNAL_HIDDEN */
Expand Down Expand Up @@ -54,7 +55,7 @@ struct bt_irk {

static inline bool bt_irk_eq(struct bt_irk const *a, struct bt_irk const *b)
{
return (memcmp(a->val, b->val, sizeof(a->val)) == 0);
return z_util_memeq(a->val, b->val, sizeof(a->val));
}

struct bt_csrk {
Expand Down
4 changes: 2 additions & 2 deletions tests/bsim/bluetooth/host/misc/disconnect/tester/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

#include <zephyr/kernel.h>
#include <zephyr/sys/util.h>
#include <zephyr/sys/byteorder.h>
#include <zephyr/sys/__assert.h>

Expand Down Expand Up @@ -190,8 +191,7 @@ static void handle_att_write(struct net_buf *buf)

static uint8_t ccc_write[2] = {0x03, 0x00};

TEST_ASSERT(buf->len == 2, "unexpected write length: %d", buf->len);
TEST_ASSERT(memcmp(buf->data, ccc_write, sizeof(ccc_write)) == 0, "bad data");
TEST_ASSERT(z_util_eq(buf->data, buf->len, ccc_write, sizeof(ccc_write)), "bad data\n");

send_write_rsp();
}
Expand Down
51 changes: 51 additions & 0 deletions tests/unit/util/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -918,4 +918,55 @@ ZTEST(util, test_utf8_lcpy_null_termination)
zassert_str_equal(dest_str, expected_result, "Failed to truncate");
}

ZTEST(util, test_z_util_eq)
{
uint8_t src1[16];
uint8_t src2[16];

bool mem_area_matching_1;
bool mem_area_matching_2;

memset(src1, 0, sizeof(src1));
memset(src2, 0, sizeof(src2));

for (size_t i = 0U; i < 16; i++) {
src1[i] = 0xAB;
src2[i] = 0xAB;
}

src1[15] = 0xCD;
src2[15] = 0xEF;

mem_area_matching_1 = z_util_eq(src1, sizeof(src1), src2, sizeof(src2));
mem_area_matching_2 = z_util_eq(src1, sizeof(src1) - 1, src2, sizeof(src2) - 1);

zassert_false(mem_area_matching_1);
zassert_true(mem_area_matching_2);
}

ZTEST(util, test_z_util_memeq)
{
uint8_t src1[16];
uint8_t src2[16];
uint8_t src3[16];

bool mem_area_matching_1;
bool mem_area_matching_2;

memset(src1, 0, sizeof(src1));
memset(src2, 0, sizeof(src2));

for (size_t i = 0U; i < 16; i++) {
src1[i] = 0xAB;
src2[i] = 0xAB;
src3[i] = 0xCD;
}

mem_area_matching_1 = z_util_memeq(src1, src2, sizeof(src1));
mem_area_matching_2 = z_util_memeq(src1, src3, sizeof(src1));

zassert_true(mem_area_matching_1);
zassert_false(mem_area_matching_2);
}

ZTEST_SUITE(util, NULL, NULL, NULL, NULL, NULL);
Loading