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.

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

Avoid nullptr derefer when constr Blockchain & tx_memory_pool (Cont.) #8924

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

jeffro256
Copy link
Contributor

@jeffro256 jeffro256 commented Jun 30, 2023

This PR replaces #8033 because it seems to be abandoned. Mine follows the same principle, but I extended the PR to enforce the usage of BlockchainAndPool throughout the codebase by making the constructors for Blockchain and tx_memory_pool private.

@jeffro256
Copy link
Contributor Author

@UkoeHB

Copy link
Contributor

@UkoeHB UkoeHB left a comment

Choose a reason for hiding this comment

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

Thanks

@selsta
Copy link
Collaborator

selsta commented Jul 1, 2023

@jeffro256 can you add DarkWingMcQuack as an author in git for the first commit?

@jeffro256
Copy link
Contributor Author

He should be the sole author of the first commit already, no?

@jeffro256
Copy link
Contributor Author

If you checkout the branch and do git log you see Author: lukas <np-hardass@protonmail.com>.

@selsta
Copy link
Collaborator

selsta commented Jul 1, 2023

@jeffro256 sorry, it showed your profile picture that's why i got confused :)

@jeffro256
Copy link
Contributor Author

Sorry fixed a really small license year typo

Copy link
Contributor

@vtnerd vtnerd left a comment

Choose a reason for hiding this comment

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

Did you consider using a std::shared_ptr instead? Or is the churn on that massive? It's would likely be annoying because one of them would have to be a std::weak_ptr to break the cycle, causing more code churn.

This patch isn't perfect, but I think it does improve the code quality a bit.

@jeffro256
Copy link
Contributor Author

One benefit of not using pointers is that you can allocate these objects on the stack, removing one layer of indirection.

@luigi1111 luigi1111 merged commit 6fc6786 into monero-project:master Aug 17, 2023
18 checks passed
@jeffro256 jeffro256 deleted the blockchain_and_pool branch October 15, 2023 06:41
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.

5 participants