Skip to content

Use nullptr instead of NULL#2487

Merged
jwillemsen merged 1 commit into
DOCGroup:masterfrom
jwillemsen:jwi-NULL
Nov 25, 2025
Merged

Use nullptr instead of NULL#2487
jwillemsen merged 1 commit into
DOCGroup:masterfrom
jwillemsen:jwi-NULL

Conversation

@jwillemsen

@jwillemsen jwillemsen commented Nov 25, 2025

Copy link
Copy Markdown
Member
* ACE/ace/Activation_Queue.cpp:
* ACE/ace/Future.cpp:
* ACE/apps/JAWS/clients/WebSTONE/src/nsapi-includes/frame/http.h:
* ACE/apps/JAWS/clients/WebSTONE/src/sysdep.h:
* ACE/tests/Network_Adapters_Test.cpp:
* TAO/tests/Bug_2234_Regression/server.cpp:
* TAO/tests/MProfile_Forwarding/client.cpp:

Summary by CodeRabbit

Release Notes

  • Documentation

    • Updated documentation, API descriptions, and error messages throughout the framework to reference modern pointer terminology and conventions.
  • Chores

    • Modernized internal code references, debug output strings, and test utilities to align with contemporary C++ standards and best practices.

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

    * ACE/ace/Activation_Queue.cpp:
    * ACE/ace/Future.cpp:
    * ACE/apps/JAWS/clients/WebSTONE/src/nsapi-includes/frame/http.h:
    * ACE/apps/JAWS/clients/WebSTONE/src/sysdep.h:
    * ACE/tests/Network_Adapters_Test.cpp:
    * TAO/tests/Bug_2234_Regression/server.cpp:
    * TAO/tests/MProfile_Forwarding/client.cpp:
@coderabbitai

coderabbitai Bot commented Nov 25, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This PR modernizes the codebase by replacing "NULL" with "nullptr" in debug messages, error output, and documentation strings across ACE and TAO libraries. Additionally, it removes associated FUZZ preprocessor directives and eliminates a NULL macro definition from a platform-specific header file.

Changes

Cohort / File(s) Summary
Debug and error message updates
ACE/ace/Activation_Queue.cpp, ACE/ace/Future.cpp, ACE/tests/Network_Adapters_Test.cpp, TAO/tests/Bug_2234_Regression/server.cpp, TAO/tests/MProfile_Forwarding/client.cpp
Changed debug output literals from "NULL" to "nullptr" and "(NON-NULL)" to "(non-nullptr)" in dump() and error-reporting functions. Removed surrounding FUZZ: disable/enable directives.
Documentation and header comments
ACE/apps/JAWS/clients/WebSTONE/src/nsapi-includes/frame/http.h
Updated http_status API documentation to refer to "nullptr" instead of "NULL" in null-pointer descriptions. Removed FUZZ comment scaffolding.
Platform macro removal
ACE/apps/JAWS/clients/WebSTONE/src/sysdep.h
Removed conditional NULL macro definition for non-Windows systems that previously defined NULL to 0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • ACE/apps/JAWS/clients/WebSTONE/src/sysdep.h: Verify that removing the NULL macro definition does not break platform-specific portability on non-Windows systems where NULL may not be predefined.
  • Cross-file consistency: Confirm all debug/error messages across the updated files have been uniformly changed to use "nullptr" terminology.

Possibly related PRs

Suggested reviewers

  • iguessthislldo
  • sonndinh

Poem

🐰 Null pointers dance with pointed cheer,
Nullptr's modern voice rings clear,
Debug strings brighten, comments fade,
A rabbit's hop through code parade! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Use nullptr instead of NULL' accurately and concisely summarizes the main objective of the pull request, which is a systematic refactoring to replace NULL references with nullptr across multiple files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
ACE/tests/Network_Adapters_Test.cpp (1)

213-217: Reactor nullptr message is fine; consider aligning remaining "NULL" strings

The updated error in the multi-remote Echo_Handler::open() correctly mentions a nullptr reactor pointer; behavior is unchanged and looks good.

For consistency with the PR goal and within this file, you may want to also:

  • Update the earlier Echo_Handler::open() overload’s "NULL pointer to reactor provided.\n" message to say nullptr.
  • Optionally modernize other diagnostics that still say "NULL ... pointer" / "NULL reactor" (e.g., the "NULL remote_addr pointer provided.\n", "NULL check_handler.\n", and "NULL reactor.\n" messages).

All of these are cosmetic; no functional issue if left as-is.

Also applies to: 138-142, 222-226, 758-771

ACE/apps/JAWS/clients/WebSTONE/src/nsapi-includes/frame/http.h (1)

117-121: http_status nullptr documentation is correct; small remaining NULL mention

The updated http_status comment correctly documents that passing nullptr for r triggers lookup of a default reason phrase; no API change.

For consistency with the rest of this header and the PR’s nullptr migration, you might optionally also change the later comment:

If you want either prefix or suffix to be skipped, use "" instead of NULL.

to say nullptr instead of NULL, or rephrase it in terms of “null pointer”, depending on how you want to present the C vs C++ story here.

Also applies to: 157-162

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b29720e and f022749.

📒 Files selected for processing (7)
  • ACE/ace/Activation_Queue.cpp (1 hunks)
  • ACE/ace/Future.cpp (1 hunks)
  • ACE/apps/JAWS/clients/WebSTONE/src/nsapi-includes/frame/http.h (1 hunks)
  • ACE/apps/JAWS/clients/WebSTONE/src/sysdep.h (0 hunks)
  • ACE/tests/Network_Adapters_Test.cpp (1 hunks)
  • TAO/tests/Bug_2234_Regression/server.cpp (4 hunks)
  • TAO/tests/MProfile_Forwarding/client.cpp (1 hunks)
💤 Files with no reviewable changes (1)
  • ACE/apps/JAWS/clients/WebSTONE/src/sysdep.h
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-02-11T10:00:08.023Z
Learnt from: jwillemsen
Repo: DOCGroup/ACE_TAO PR: 870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator, not i. For Linux abstract paths, i is used to skip the first character (i=1) when both paths are empty, but the -1 is still needed to handle the null terminator correctly.

Applied to files:

  • TAO/tests/Bug_2234_Regression/server.cpp
📚 Learning: 2025-02-11T10:00:08.023Z
Learnt from: jwillemsen
Repo: DOCGroup/ACE_TAO PR: 870
File: ACE/ace/UNIX_Addr.inl:39-39
Timestamp: 2025-02-11T10:00:08.023Z
Learning: In ACE_UNIX_Addr's equality operator, the comparison length should be sizeof(sun_path) - 1 to exclude the null terminator. For Linux abstract paths, while i=1 is used to skip the first character, using i instead of -1 in the length calculation would be incorrect as it would either include the null terminator (for regular paths) or miss the last valid character (for abstract paths).

Applied to files:

  • TAO/tests/Bug_2234_Regression/server.cpp
⏰ 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: Codacy Static Code Analysis
🔇 Additional comments (4)
TAO/tests/MProfile_Forwarding/client.cpp (1)

50-55: Updated objref null diagnostic looks good

Log text now correctly mentions nullptr; behavior and null check remain unchanged and correct.

ACE/ace/Activation_Queue.cpp (1)

25-30: dump() output now consistent with nullptr usage

ACE_Activation_Queue::dump() now reports (nullptr) when queue_ is null; this is consistent with the rest of the codebase and purely cosmetic.

TAO/tests/Bug_2234_Regression/server.cpp (1)

100-105: String-parameter nullptr diagnostics are updated and clear

All four messages now explicitly mention nullptr in the null-string cases for parameters a and c; exception paths and validation logic are unchanged and correct.

Also applies to: 116-120, 183-188, 199-203

ACE/ace/Future.cpp (1)

38-43: ACE_Future_Rep::dump() nullptr wording is consistent and correct

The dump now distinguishes value_ being set vs. null using (non-nullptr) and (nullptr); this aligns with modern C++ pointer terminology and does not affect behavior.

@jwillemsen jwillemsen merged commit 3abc747 into DOCGroup:master Nov 25, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant