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

deps: ntlmclient: disable implicit fallthrough warnings #5112

Merged
merged 1 commit into from
Jun 14, 2019

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Jun 14, 2019

The ntlmclient dependency has quite a lot of places with implicit
fallthroughs. As at least modern GCC has enabled warnings on
implicit fallthroughs by default, the developer is greeted with a
wall of warnings when compiling that dependency.

Disable implicit fallthrough warnings for ntlmclient to fix this
issue.

The ntlmclient dependency has quite a lot of places with implicit
fallthroughs. As at least modern GCC has enabled warnings on
implicit fallthroughs by default, the developer is greeted with a
wall of warnings when compiling that dependency.

Disable implicit fallthrough warnings for ntlmclient to fix this
issue.
@tiennou
Copy link
Contributor

tiennou commented Jun 14, 2019

LGTM (nightlies-only bionic builds failed because of that). Though I see

/src/deps/ntlmclient/unicode_builtin.c: At top level:
cc1: error: unrecognized command line option '-Wno-documentation-deprecated-sync' [-Werror]
cc1: all warnings being treated as errors

Hopefully this is just a confused GCC ?

@pks-t
Copy link
Member Author

pks-t commented Jun 14, 2019

Hopefully this is just a confused GCC ?

Yeah, I don't really know where that comes from, but we had that issue since several months already. I've noticed it multiple times, but I've got no idea when exactly it pops up and I didn't think it important enough to really care

@ethomson
Copy link
Member

I'm happy enough to fix this in ntmlclient itself, if you'd prefer. My goal in building ntlmclient was for it to basically be part of libgit2 (in terms of codestyle, etc) but available for others as well. 🤷‍♂

@pks-t
Copy link
Member Author

pks-t commented Jun 14, 2019

@ethomson: how about merging this as a short-term fix with the mid-term goal being to fix it in ntlmclient?

@ethomson ethomson merged commit bed33a6 into libgit2:master Jun 14, 2019
@pks-t pks-t deleted the pks/ntlmclient-implicit-fallthrough branch June 14, 2019 09:54
@ethomson
Copy link
Member

Welp, I tried going through unicode_builtin to add /* fallthrough */ comments, but gcc appears confused about the switch-within-a-switch, so... I think that no-implicit-fallthrough is probably the best approach. 🙇

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

3 participants