Skip to content

Refactor NULL with nullptr in brpc/policy - #3458

Merged
wasphin merged 2 commits into
apache:masterfrom
chenBright:brpc_policy_nullptr
Aug 18, 2026
Merged

Refactor NULL with nullptr in brpc/policy#3458
wasphin merged 2 commits into
apache:masterfrom
chenBright:brpc_policy_nullptr

Conversation

@chenBright

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: resolve

Problem Summary:

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

@chenBright
chenBright force-pushed the brpc_policy_nullptr branch from 630d735 to c17f06d Compare August 16, 2026 16:12
@chenBright
chenBright force-pushed the brpc_policy_nullptr branch from c17f06d to b15e071 Compare August 17, 2026 02:21
}
if (first == 0xfb) {
// 0xFB is the lenenc NULL marker, not a length prefix. Report NULL
// 0xFB is the lenenc nullptr marker, not a length prefix. Report nullptr

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some NULLs in the comments are protocol-level terms, not C++
null pointers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


AutoConcurrencyLimiter* AutoConcurrencyLimiter::New(const AdaptiveMaxConcurrency&) const {
return new (std::nothrow) AutoConcurrencyLimiter;
return new AutoConcurrencyLimiter;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if this change is intentional or accidental?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are about 5 places with this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's intentional. If new fails, it's basically unrecoverable; just throw a crash exception.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Better to split this into different commits.

@wasphin

wasphin commented Aug 18, 2026

Copy link
Copy Markdown
Member

LGTM

@wasphin
wasphin merged commit 3366074 into apache:master Aug 18, 2026
25 of 26 checks passed
@chenBright
chenBright deleted the brpc_policy_nullptr branch August 18, 2026 14:29
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.

2 participants