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

Powellsr/fix/debug attach handling #473

Merged
merged 6 commits into from
Nov 10, 2024

Conversation

powellsr
Copy link
Contributor

Mostly followed MPQC version, didn't use HAVE_SYSTEM macro, as it doesn't appear to be defined in TA code.
system_retvalue, ~ bug.cpp:240 may not be used correctly in MPQC. It's defined as zero and never changed; the system call return value is assigned to another var, which is cast to void (which I learned silences compiler warnings, is there a reason for this?

@powellsr powellsr requested a review from evaleev September 16, 2024 15:38
@evaleev evaleev force-pushed the powellsr/fix/debug-attach-handling branch from 3887e36 to 99decc1 Compare November 10, 2024 17:34
@evaleev
Copy link
Member

evaleev commented Nov 10, 2024

Mostly followed MPQC version, didn't use HAVE_SYSTEM macro, as it doesn't appear to be defined in TA code.

HAVE_SYSTEM is no longer needed since std::system is standard.

system_retvalue, ~ bug.cpp:240 may not be used correctly in MPQC. It's defined as zero and never changed; the system call return value is assigned to another var, which is cast to void (which I learned silences compiler warnings, is there a reason for this?

Not sure which code you are looking at, this looks legit: https://github.com/ValeevGroup/mpqc4/blob/master/src/mpqc/util/misc/bug.cpp#L266

@evaleev evaleev merged commit 87664ae into master Nov 10, 2024
9 checks passed
@evaleev evaleev deleted the powellsr/fix/debug-attach-handling branch November 10, 2024 18:30
@powellsr
Copy link
Contributor Author

Right, we'd fixed that, maybe the commit ended up in another branch, but that was an issue we handled in a meeting at some point

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