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

Fix ' warning: delete called on non-final 'PeerLogicValidation' that has virtual functions but non-virtual destructor' #2363

Closed
wants to merge 2 commits into from

Conversation

Duality-CDOO
Copy link

@Duality-CDOO Duality-CDOO commented Oct 22, 2018

peerLogic.reset(); in init.cpp calls on delete on PeerLogicValidation in net_processing and PeerLogicValidation is non-final.

…has virtual functions but non-virtual destructor'
@Duality-CDOO
Copy link
Author

The failures on Jenkins do not seem to be related to my code change.

@UdjinM6
Copy link

UdjinM6 commented Oct 23, 2018

Thanks but this one is going to conflict with bitcoin#12680 on future backports since it basically replicates (only) a part of it.

@Duality-CDOO
Copy link
Author

Duality-CDOO commented Oct 23, 2018

If you wish @UdjinM6 I can update the entire PeerLogicValidation and add NetEventsInterface.

EDIT: That would update signals and schedular use too.

@UdjinM6
Copy link

UdjinM6 commented Oct 23, 2018

You mean backporting bitcoin#10756 first (or is there anything else)? 10756 looks pretty straightforward on the first sight (and I also like the fact that it drops some boost dependencies), though there might be quite a few merge conflicts since it changes the code all over the place actually. I'm not sure if we really want to touch any critical parts atm just because of some warnings tbh, probably better to postpone non-critical/non-functional fixes like this to future "backport packs". Thoughts @codablock ?

@codablock
Copy link

Agree with @UdjinM6. Not the right time to backport this.

Also, if we want to backport cosmetical stuff from Bitcoin, then I'd always prefer to backport in the same order as stuff was merged into Bitcoin. When I backported the changes from Bitcoin 0.13+0.14, the merge resolution which was needed due to previous out-of-order backporting was about 90% of the work (and it took weeks).

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.

3 participants