utf8_to_uv_msgs: Do some code cleanup #22819
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This case: has two occurrences of the same statement, within two different conditionals. But the case: doesn't get executed unless at least one of those conditionals is known to be true. Therefore the statement is guaranteed to be executed at least once; no need to have two copies.
Processing the overlong malformation needed to be last because it likely would overwrite the calculated UV. Other cases also overwrote that. This is unnecessarily brittle, as we can simply store the UV before processing any cases, and then refer to that copy.
This will allow some simplification in future commits
These are three case: statements in a switch in a loop. The code after the switch is executed only if necessary. By complementing some conditionals, we know that that common code isn't going to be used and we can use 'continue' to avoid it and go directly to the next iteration.
These case: statements have virtually identical logic. This commit combines that into a block before the switch() and then each case: does the part that is different from the others.
The new code is equivalent to the old, but is missing a conditional
The previous commit removed two levels of blocks for these case: statements. We can outdent correspondingly And remove a no-longer relevant comment
This only is needed if a problem was found.
The input parameter can be used for several instances before this copy gets incremented.
By deferring to the end of the function the setting of a variable returned to the caller, we can eliminate one copy of that setting
This makes these two variables always contain the value their names indicate.
This is a small detail, but this moves this assert to after a conditional that would exclude it. That is, if the conditional is true, the assert is pointless. So move the assert to where we know the conditional is false.
C99 allows us to declare anywhere; so move these to where its more logical. It also makes sure some variables are initialized before the goto that jumps to the end of the program, and which currently doesn't rely on these values, but could be changed to do so someday without the coder realizing it. This prevents a problem in case that happens.
These showed up running gcc on a 32-bit build
This was introduced in a rebasing error
khwilliamson
force-pushed
the
utf8_to_uv_msgs
branch
from
23ee4f2
to
f88c1ea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should not change behavior.
It moves declarations and initializations closer to first use. The biggest change is that several cases in a switch have virtually identical logic, which this extracts out into common code