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
HTTP: fix empty-body warning #12262
HTTP: fix empty-body warning #12262
Conversation
Ref, this fixes one of the 4 warnings reported in #12228. |
lib/http.c
Outdated
@@ -1144,7 +1144,7 @@ CURLcode Curl_http_input_auth(struct Curl_easy *data, bool proxy, | |||
} | |||
} | |||
#else | |||
; | |||
{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing this is an unusual construct for curl code, can you add a comment here explaining why plain ;
is not enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply put it: Compilers. gcc warns about orphaned ;
in the source code. This warning is also enabled by the -Wextra
flag. I'm not sure about the reason, I would guess it prevents programming errors like this:
if (foo);
do_stuff();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and I want that as a comment in the code so that future readers understand that is the reason why this construct is used...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misread, will add!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I personally find this macro spaghetti quite hard to understand. Would you be fine if I refactor this into static functions too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this: (pseudocode)
static bool handle_ntlm(const char *auth, ...) {
#ifdef USE_NTLM
if (!checkprefix("Negotiate", auth) || !is_valid_auth_separator(auth[9])) {
return false;
}
// NTLM auth code
return true;
#else
(void)auth;
// ...
return false;
#endif
}
// ...
CURLcode Curl_http_input_auth(struct Curl_easy *data, bool proxy,
const char *auth) /* the first non-space */
{
// ...
if (handle_ntlm(auth, ...)) {
} else if (handle_basic(auth, ...)) {
} else if (handle_...(auth, ...)) {
} else ...
The compiler would still eliminate empty functions, but I find it less noisy to read as the macros don't need to workaround syntactic issues. Do you think this could help readability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think it would. Reducing ifdef-mazes is a worthy effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR is finished as is, I'll open a new one for the refactoring. (I know myself, my attention is a squirrel.)
9dcd9cb
to
9418089
Compare
This change fixes a compiler warning with gcc-12.2.0 when `-DCURL_DISABLE_BEARER_AUTH=ON` is used. /home/tox/src/curl/lib/http.c: In function 'Curl_http_input_auth': /home/tox/src/curl/lib/http.c:1147:12: warning: suggest braces around empty body in an 'else' statement [-Wempty-body] 1147 | ; | ^
9418089
to
0547a7b
Compare
This change fixes a compiler warning with gcc-12.2.0 when `-DCURL_DISABLE_BEARER_AUTH=ON` is used. /home/tox/src/curl/lib/http.c: In function 'Curl_http_input_auth': /home/tox/src/curl/lib/http.c:1147:12: warning: suggest braces around empty body in an 'else' statement [-Wempty-body] 1147 | ; | ^ Closes curl#12262
This change fixes a compiler warning with gcc-12.2.0 when
-DCURL_DISABLE_BEARER_AUTH=ON
is used.This is useful as it fixes an issue when libcurl is compiled with
-Werror
.