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
Curl_initinfo not called consistently #1103
Conversation
initinfo is called on perform not init. I don't think there would be any ramifications to calling it in init as well. as far as duphandle goes I'm pretty sure it's supposed to be like that. duphandle copies the options not the info. |
ah I just re-read what you wrote. you are saying since the info is not copied on duphandle it should initinfo. ok. |
Yeah. The inconsistency I was seeing was because 22cfeac started calling initinfo on reset, so a handle which was reset would have different values than a handle which was freshly initialized or freshly duplicated. I only noticed because we were calling reset on new handles automatically (changed in facebook/hhvm@7f4b85b) but the inconsistency seemed worth raising here. :-) |
- Call Curl_initinfo on init and duphandle. Prior to this change the statistical and informational variables were simply zeroed by calloc on easy init and duphandle. While zero is the correct default value for almost all info variables, there is one where it isn't (filetime initializes to -1). Bug: curl#1103 Reported-by: Neal Poole
@jay, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bagder, @dfandrich and @captain-caveman2k to be potential reviewers. |
Ok I turned this into a PR with what I think we should do, someone please review. |
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.
Ship it!
It'd be cool to add a test... |
- Call Curl_initinfo on init and duphandle. Prior to this change the statistical and informational variables were simply zeroed by calloc on easy init and duphandle. While zero is the correct default value for almost all info variables, there is one where it isn't (filetime initializes to -1). Bug: #1103 Reported-by: Neal Poole
Done, landed in 4564636. |
With the latest libcurl (v7.51.0) I ran into an odd behavior caused by Curl_initinfo not being called/copied over consistently. As a result, CURLINFO_FILETIME may oscillate between 0 and -1 unexpectedly.
curl_easy_init
,Curl_initinfo
is not called. That meansCURLINFO_FILETIME
will be set to 0.curl_easy_reset
on that handle,Curl_initinfo
is called. This behavior is new starting in 22cfeac. That meansCURLINFO_FILETIME
will be set to -1.curl_easy_duphandle
on that handle,Curl_initinfo
is not called and the info is never set. That meansCURLINFO_FILETIME
will be set to 0.This came up because of a test in HHVM (PHP interpreter) that tried to verify that two handles had the same info after calling
curl_easy_duphandle
.curl_init
in HHVM would callcurl_easy_init
andcurl_easy_reset
internally, soCURLINFO_FILETIME
would be set to -1. If you calledcurl_easy_duphandle
the new handle would haveCURLINFO_FILETIME
set to 0.It seems like for consistency the code could be doing a couple things to make the behavior consistent:
Curl_initinfo
insidecurl_easy_init
Curl_initinfo
incurl_easy_duphandle
(if we don't care about not exactly duplicating the handle) or copying the data explicitly from one handle to the other.