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

Update cellVoice.cpp #6857

Merged
merged 1 commit into from Oct 24, 2019
Merged

Update cellVoice.cpp #6857

merged 1 commit into from Oct 24, 2019

Conversation

JohnHolmesII
Copy link
Contributor

Caused warnings. Not sure what the actual intention was, this may need to be inverted.

@elad335
Copy link
Contributor

elad335 commented Oct 24, 2019

the intention was to compare if equals to 0.

@JohnHolmesII
Copy link
Contributor Author

Uh, yes I could tell that there was a comparison to 0. I'm saying I am as uncertain as the compiler is what was supposed to be compared to 0, and which state was supposed to return an error.

Consider each of the 3
if (!manager->ports.erase((u16)portId) == 0)
if (!(manager->ports.erase((u16)portId) == 0))
if ((!manager->ports.erase((u16)portId)) == 0)

These 3 read very differently (and some give warnings). The way I read the code, it seemed to me that it was a simple return code check, so I patched it as such. If I misinterpreted, and the function returning 0 is an error, then the condition will need to be inverted.

Caused warnings. Not sure what the actual intention was, this may need to be inverted.
This commit assumes that erase() returning 0 is a sign that deletion
failed, or that there was corruption. There should be a port there.
@JohnHolmesII
Copy link
Contributor Author

I think the last push makes more sense, having looked at the docs for erase()

@Megamouse
Copy link
Contributor

Megamouse commented Oct 24, 2019

As far as I understood topology is returned if no port with such ID was registered. So if n erased is 0 it should be an error ?!

@elad335
Copy link
Contributor

elad335 commented Oct 24, 2019

yes

@Nekotekina Nekotekina merged commit 0842724 into RPCS3:master Oct 24, 2019
@JohnHolmesII JohnHolmesII deleted the patch-1 branch April 7, 2020 03:13
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.

None yet

4 participants