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

winterm: fix GetStdFile() falltrough #32

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Conversation

thaJeztah
Copy link
Contributor

Commit 3ee0038 (#31) updated the switch with the intent to handle either syscall.XX or golang.org/x/sys/win.XX values.

However, Go "switch" does not have an implicit "fallthrough" behavior, and either requires the "fallthrough" keyword to be used, or to combine values in a single "case".

For example, the following:

for _, v := range []int{1, 2, 3, 4, 5, 6, 7} {
    result := "unknown"
    switch v {
    case 1: // doesn't fall-through, so this is an empty "case"
    case 2:
        result = "found one or two"
    case 3:
        fallthrough // falls through to "case 4"
    case 4:
        result = "found three or four"
    case 5, 6: // matches either 5 or 6
        result = "found five or six"
    default:
        result = "found something else"
    }
    fmt.Printf("%d: %s\n", v, result)
}

Will print:

1: unknown
2: found one or two
3: found three or four
4: found three or four
5: found five or six
6: found five or six
7: found something else

This patch combines "equal" values in a single case within the switch to
fix this issue.

Commit 3ee0038 updated the switch
with the intent to handle either syscall.XX or golang.org/x/sys/win.XX
values.

However, Go "switch" does not have an implicit "fallthrough" behavior,
and either requires the "fallthrough" keyword to be used, or to combine
values in a single "case".

For example, the following:

    for _, v := range []int{1, 2, 3, 4, 5, 6, 7} {
        result := "unknown"
        switch v {
        case 1: // doesn't fall-through, so this is an empty "case"
        case 2:
            result = "found one or two"
        case 3:
            fallthrough // falls through to "case 4"
        case 4:
            result = "found three or four"
        case 5, 6: // matches either 5 or 6
            result = "found five or six"
        default:
            result = "found something else"
        }
        fmt.Printf("%d: %s\n", v, result)
    }

Will print:

    1: unknown
    2: found one or two
    3: found three or four
    4: found three or four
    5: found five or six
    6: found five or six
    7: found something else

This patch combines "equal" values in a single case within the switch to
fix this issue.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Contributor Author

@jstarks @n4j PTAL

@jstarks
Copy link
Collaborator

jstarks commented Jun 8, 2021

Whoops. I should have looked more closely at the original change. Thanks, LGTM.

@jstarks jstarks merged commit 2377c96 into Azure:master Jun 8, 2021
@thaJeztah thaJeztah deleted the fix_switch branch June 8, 2021 22:42
@thaJeztah
Copy link
Contributor Author

Definitely easy to miss, as other languages do fall through, and it doesn't look "wrong"

Thanks for the merge!

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