-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Fix build with ALTSVC enabled and COOKIES disabled #3717
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
Conversation
ALTSVC requires Curl_get_line which is defined in lib/cookie.c inside a #if check of HTTP and COOKIES. That makes Curl_get_line undefined if COOKIES is disabled. This is a workaround to define Curl_get_line unconditionally.
This is just the workaround I committed for FreeBSD ports tree. You might want to move it to a new place, e.g. lib/curl_get_line.c and lib/curl_get_line.h. |
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.
I'm more inclined to move this into a generic utilities file, or a file-specific utilities file. It's not very logical to keep it cookie.c
now that it has multiple consumers.
Assuming there is buy-in from other maintainers, I'm happy to pick that up in case you're not interested in pursuing that patch.
I agree with you @danielgustafsson. I was mostly just lazy when I left it there in my alt-svc patch... |
Cool, let's do it. @sunpoet do you want to pursue this patch? If not then I'm fine to pick it up. |
It's a better way to fix build with ALTSVC enabled and COOKIES disabled.
I've prepared the patch which moves Curl_get_line to lib/curl_get_line.[ch]. |
lib/cookie.h
Outdated
@@ -24,6 +24,7 @@ | |||
#include "curl_setup.h" | |||
|
|||
#include <curl/curl.h> | |||
#include "curl_get_line.h" |
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.
This isn't used in the header, is it? I'd rather move it to the implementation file.
|
||
#include "curl_setup.h" | ||
|
||
#include "curl_get_line.h" |
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.
This should also include the following headers after curl_get_line.h
:
#include "curl_memory.h"
/* The last #include file should be: */
#include "memdebug.h"
@sunpoet, are you up to taking this all the way to the finishing line? |
Suggested by @MarcelRaad and @danielgustafsson
I just committed the changes suggested by @MarcelRaad and @danielgustafsson. Thanks! |
It is a followup fix of 67ba6c3.
Pushed squashed and with some minor editorialization to the commit message, thanks for contributing! |
Thanks! |
ALTSVC requires Curl_get_line which is defined in lib/cookie.c inside a #if
check of HTTP and COOKIES. That makes Curl_get_line undefined if COOKIES is
disabled. This is a workaround to define Curl_get_line unconditionally.