Fix setValue() using sizeof instead of strlen for char array arguments

[h2zero]

Fixes #376

Summary by CodeRabbit

• Bug Fixes
  • Corrected handling of character array attribute values to properly calculate null-terminated string lengths instead of treating entire arrays as raw byte buffers. This resolves inconsistent behavior across C++ standards and ensures accurate attribute value assignments when working with string-based attributes.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified NimBLEAttValue.h to add template specializations and if constexpr branches that detect char array types and calculate length via strnlen() instead of treating them as raw byte blobs, addressing the inability to accept char* and char[N] inputs.

Changes

Cohort / File(s)	Summary
Template Specialization for Char Arrays src/NimBLEAttValue.h	Added new setValue(const T&) template overload for C++<17 targeting char array types using std::is_array and element-type detection. Extended C++17+ if constexpr chain to detect and handle char[N] arrays via strnlen(s, sizeof(s)) instead of raw sizeof(T) copying. Prevents char* and char[N] from being treated as uninitialized byte blobs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

char arrays now find their true length,
No more bytes of untamed strength,
strnlen counts with careful might,
Template magic shines so bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main fix: addressing the improper use of sizeof instead of strlen for char array arguments in the setValue() method.
Linked Issues check	✅ Passed	The PR successfully resolves issue #376 by enabling setValue() to properly accept char* arguments through template overloads and constexpr logic that correctly computes string length.
Out of Scope Changes check	✅ Passed	All changes in NimBLEAttValue.h are directly scoped to fixing the char array/char* handling in setValue(), with no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch bugfix/char-data-len

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/NimBLEAttValue.h (1)

357-359: Minor inconsistency between the two code paths, savvy?

Arr, the C++14 path at line 270 uses sizeof(T), but this here C++17 path uses sizeof(s). Both be givin' ye the same result fer a reference to an array, so this ain't causin' any scurvy bugs. But fer the sake of keepin' yer code shipshape and consistent across both paths, ye might consider usin' sizeof(T) here as well.

🏴‍☠️ Proposed fix for consistency

         } else if constexpr (std::is_array<T>::value &&
                              std::is_same<typename std::remove_extent<T>::type, char>::value) {
-            return setValue(reinterpret_cast<const uint8_t*>(s), strnlen(s, sizeof(s)));
+            return setValue(reinterpret_cast<const uint8_t*>(s), strnlen(s, sizeof(T)));
         } else {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEAttValue.h` around lines 357 - 359, In NimBLEAttValue.h, the C++17
template branch that handles char arrays uses sizeof(s) when calling strnlen;
change this to use sizeof(T) for consistency with the C++14 path and to
correctly reflect the array type size in the branch around the reinterpret_cast
and setValue call (i.e., update the branch that returns
setValue(reinterpret_cast<const uint8_t*>(s), strnlen(s, sizeof(s))) to use
sizeof(T) instead of sizeof(s)).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/NimBLEAttValue.h`:
- Around line 357-359: In NimBLEAttValue.h, the C++17 template branch that
handles char arrays uses sizeof(s) when calling strnlen; change this to use
sizeof(T) for consistency with the C++14 path and to correctly reflect the array
type size in the branch around the reinterpret_cast and setValue call (i.e.,
update the branch that returns setValue(reinterpret_cast<const uint8_t*>(s),
strnlen(s, sizeof(s))) to use sizeof(T) instead of sizeof(s)).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3e4d67af-eafe-461b-88f2-99039a73690c

📥 Commits

Reviewing files that changed from the base of the PR and between 8bb9578 and 1646d27.

📒 Files selected for processing (1)

• src/NimBLEAttValue.h