Skip to content

Windows compilation: ensure statusCodeToString has no control paths that do not return#10777

Merged
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
greenhouse-org:http-status-return-paths
Apr 15, 2020
Merged

Windows compilation: ensure statusCodeToString has no control paths that do not return#10777
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
greenhouse-org:http-status-return-paths

Conversation

@sunjayBhatia
Copy link
Member

@sunjayBhatia sunjayBhatia commented Apr 14, 2020

Description: Fix Windows compilation to ensure all control paths return, any unmatched status code that is unexpected is taken care of by adding NOT_REACHED_GCOVR_EXCL_LINE to induce a PANIC, related to #10550
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

@sunjayBhatia
Copy link
Member Author

sunjayBhatia commented Apr 14, 2020

If an exception is more desirable here let us know, we chose to assert as there is no usage of this class other than in tests (also we can add tests as needed as well).

@sunjayBhatia
Copy link
Member Author

sunjayBhatia commented Apr 14, 2020

Pulled out in a separate PR instead of #10542 to enable more discussion and as it is a minor change for Linux semantics

Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>
Co-authored-by: William A Rowe Jr <wrowe@pivotal.io>
Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
@wrowe
Copy link
Contributor

wrowe commented Apr 15, 2020

@asraa and @yanavlasov this is the patch to address the missing default control in PR10550 if someone can assume review of this PR.

same result, macro adds a PANIC directly

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: William A Rowe Jr <wrowe@pivotal.io>
@yanavlasov yanavlasov self-assigned this Apr 15, 2020
yanavlasov
yanavlasov previously approved these changes Apr 15, 2020
Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>
@sunjayBhatia
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #10777 (comment) was created by @sunjayBhatia.

see: more, trace.

@mattklein123 mattklein123 merged commit 44d0eb6 into envoyproxy:master Apr 15, 2020
@wrowe wrowe deleted the http-status-return-paths branch April 15, 2020 19:26
nareddyt added a commit to nareddyt/envoy that referenced this pull request Apr 23, 2020
Description: PR envoyproxy#10777 introduces `NOT_REACHED_GCOVR_EXCL_LINE`, but this macro is defined in `assert.h`
Signed-off-by: Teju Nareddy <nareddyt@google.com>
mattklein123 pushed a commit that referenced this pull request Apr 23, 2020
PR #10777 introduces `NOT_REACHED_GCOVR_EXCL_LINE`, but this macro is 
defined in `assert.h`

Signed-off-by: Teju Nareddy <nareddyt@google.com>
@sunjayBhatia sunjayBhatia mentioned this pull request Apr 24, 2020
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.

5 participants