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.

Sign up for GitHub

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

Jump to bottom

NoNewGlobals for MemBlob::Stats #1749

Closed
wants to merge 2 commits into from
Closed

NoNewGlobals for MemBlob::Stats #1749

wants to merge 2 commits into from

Conversation

kinkie
Copy link
Contributor

@kinkie kinkie commented Mar 19, 2024 •
edited by rousskov
Loading

Detected by Coverity. CID 1554656: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).

Also update MemBlobStats initialization.

yadij
yadij requested changes Mar 19, 2024
View reviewed changes
src/sbuf/MemBlob.h Outdated Show resolved Hide resolved
@kinkie kinkie requested a review from yadij March 19, 2024 12:45
@kinkie kinkie added the S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box label Mar 19, 2024
rousskov
rousskov reviewed Mar 19, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

I will need to come back to this to do the usual cleanup.

@rousskov rousskov self-requested a review March 19, 2024 13:16
yadij
yadij requested changes Mar 19, 2024
View reviewed changes
src/sbuf/MemBlob.h Outdated Show resolved Hide resolved
@yadij yadij added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Mar 19, 2024
@kinkie kinkie requested a review from yadij March 19, 2024 16:08
@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Mar 19, 2024
rousskov
rousskov requested changes Mar 20, 2024
View reviewed changes
src/sbuf/MemBlob.cc Outdated Show resolved Hide resolved
src/sbuf/MemBlob.cc Outdated Show resolved Hide resolved
src/sbuf/MemBlob.cc Outdated Show resolved Hide resolved
src/sbuf/MemBlob.cc Outdated Show resolved Hide resolved
@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Mar 20, 2024
@kinkie kinkie removed the S-waiting-for-author author action is expected (and usually required) label Mar 21, 2024
@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Mar 21, 2024
@rousskov

This comment was marked as resolved.

Sign in to view
@kinkie
NoNewGlobals for MemBlob::Stats
d805604 
Detected by Coverity. CID 1554656: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).

Also update MemBlobStats initialization
@kinkie

This comment was marked as resolved.

Sign in to view
@kinkie
Copy link
Contributor Author

kinkie commented Mar 22, 2024

Run-tested; excerpt from http://localhost:3128/squid-internal-mgr/sbuf :

MemBlob created: 2115
MemBlob alive: 1297
MemBlob append calls: 3020
MemBlob currently allocated size: 62688

@kinkie kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box S-waiting-for-author author action is expected (and usually required) labels Mar 22, 2024
@kinkie kinkie requested a review from rousskov March 22, 2024 03:10
rousskov
rousskov approved these changes Mar 22, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Thank you for polishing this up!

@rousskov rousskov mentioned this pull request Mar 24, 2024
Open
@kinkie

This comment was marked as resolved.

Sign in to view
@squid-anubis squid-anubis added M-failed-other https://github.com/measurement-factory/anubis#pull-request-labels and removed M-failed-other https://github.com/measurement-factory/anubis#pull-request-labels labels Apr 5, 2024
@kinkie kinkie requested a review from rousskov April 5, 2024 10:19
@rousskov rousskov removed their request for review April 5, 2024 16:38
rousskov
rousskov reviewed Apr 5, 2024
View reviewed changes
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

rousskov approved these changes on Mar 22, 2024

kinkie merged branch 'master' into coverity-cid-1554656

kinkie kinkie requested a review from rousskov on April 5, 2024 06:19

rousskov removed their request for review on April 5, 2024 12:38

@kinkie, please do not request my re-review after uneventful/clean mergers of master branch1 unless there is something specific you want me to check or respond to. In the latter case, please mention what that is. Otherwise, I am facing a PR that, on the surface, should be the same as the one I have reviewed already, but with a new pending review request; I do not want to re-review everything from scratch unless there is a compelling reason to do that. This recommendation applies to the PRs I have approved (like this one), but it is not specific to such PRs.

Footnotes

  1. And merging master into feature branch should only be performed if necessary from PR code/review point of view. For example, these merges should not be abused for triggering or restarting CI tests. This footnote does not imply that an unwanted merger has happened in this PR. ↩

@yadij yadij added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Apr 8, 2024
squid-anubis pushed a commit that referenced this pull request Apr 8, 2024
@kinkie @squid-anubis
NoNewGlobals for MemBlob::Stats (#1749)
9e6c696 
Detected by Coverity. CID 1554656: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).

Also update MemBlobStats initialization.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Apr 8, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Apr 8, 2024
kinkie added a commit to kinkie/squid that referenced this pull request Apr 9, 2024
@kinkie
NoNewGlobals for MemBlob::Stats (squid-cache#1749)
2429915 
Detected by Coverity. CID 1554656: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).

Also update MemBlobStats initialization.
kinkie added a commit to kinkie/squid that referenced this pull request Apr 9, 2024
@kinkie
NoNewGlobals for MemBlob::Stats (squid-cache#1749)
468faa7 
Detected by Coverity. CID 1554656: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).

Also update MemBlobStats initialization.
kinkie added a commit to kinkie/squid that referenced this pull request Apr 14, 2024
@kinkie
NoNewGlobals for MemBlob::Stats (squid-cache#1749)
5516fe4 
Detected by Coverity. CID 1554656: Initialization or destruction
ordering is unspecified (GLOBAL_INIT_ORDER).

Also update MemBlobStats initialization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rousskov rousskov rousskov approved these changes

@yadij yadij yadij approved these changes

Assignees
No one assigned
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kinkie @rousskov @yadij @squid-anubis