Skip to content

Comments

minor warning fix#1415

Merged
JaylinYu merged 2 commits intomainfrom
jaylin/dev
Jan 14, 2026
Merged

minor warning fix#1415
JaylinYu merged 2 commits intomainfrom
jaylin/dev

Conversation

@JaylinYu
Copy link
Member

@JaylinYu JaylinYu commented Jan 14, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Fixed handling of NULL bridge names in MQTT configuration to prevent potential errors and improve stability.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Jaylin <jaylin@emqx.io>
Signed-off-by: Jaylin <jaylin@emqx.io>
@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

The PR makes two targeted improvements to MQTT protocol handling: removes a log_error statement from the nmq timer callback in nmq_mqtt.c, and adds NULL-safe assignment checking to bridge_name in mqtt_public.c to prevent NULL dereference.

Changes

Cohort / File(s) Summary
MQTT Protocol Logging & Configuration
src/sp/protocol/mqtt/nmq_mqtt.c, src/supplemental/mqtt/mqtt_public.c
Removed debug logging during pipe removal in timer callback; added NULL-safe conditional assignment for bridge_name string duplication to prevent dereferencing NULL values

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • fix #1406 #1408: Modifies the same nmq_mqtt.c timer callback and pipe/session expiry path where the log statement was removed.
  • port a session keeping fix to sdv #1409: Also touches mqtt session handling and sqlite bridge_name allocation logic in both modified files within this PR.

Suggested reviewers

  • wanghaEMQ

Poem

🐰 A log line fades away so bright,
NULL checks now hold the bridge just right,
Two tiny tweaks, so clean and lean,
The safest MQTT you've ever seen! 🌿

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is completely missing; the required template format with issue reference and change description is not provided. Add a PR description following the template format, including the issue number, synopsis, and comments explaining the changes made.
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 (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'minor warning fix' accurately reflects the main changes in the PR, which remove a log_error line and implement a NULL-safe pattern to address compiler warnings.

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e8f3bf and 4bfd61f.

📒 Files selected for processing (2)
  • src/sp/protocol/mqtt/nmq_mqtt.c
  • src/supplemental/mqtt/mqtt_public.c
💤 Files with no reviewable changes (1)
  • src/sp/protocol/mqtt/nmq_mqtt.c
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T11:06:33.930Z
Learnt from: wanghaEMQ
Repo: nanomq/NanoNNG PR: 1384
File: src/sp/protocol/mqtt/auth_http.c:411-418
Timestamp: 2025-12-18T11:06:33.930Z
Learning: In C code using nng_id_map_foreach2 (as in NanoNNG), the callback receives the key as a void* pointing to the actual key value (e.g., a uint64_t*). Do not cast the void* directly to a value type; instead dereference properly, e.g., uint64_t key = *(uint64_t*)k, to obtain the real key. This avoids misinterpretation of the pointer and ensures correct key retrieval in all callbacks that pass a pointer to the key value.

Applied to files:

  • src/supplemental/mqtt/mqtt_public.c
🧬 Code graph analysis (1)
src/supplemental/mqtt/mqtt_public.c (1)
src/nng.c (1)
  • nng_strdup (106-110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (1)
src/supplemental/mqtt/mqtt_public.c (1)

1141-1149: LGTM! Correct NULL-safe guard for bridge_name.

The ternary check prevents passing NULL to nng_strdup, which would cause undefined behavior. The corresponding nng_strfree in nng_mqtt_free_sqlite_opt should handle NULL gracefully, so this change is safe.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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

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

@JaylinYu JaylinYu merged commit 28c688c into main Jan 14, 2026
2 of 16 checks passed
@JaylinYu JaylinYu deleted the jaylin/dev branch January 14, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant