[Tarantool-patches] [PATCH v4 2/4] http: remove redundant & incorrect case for curl_request code
Alexander Turenko
alexander.turenko at tarantool.org
Tue Oct 29 02:42:41 MSK 2019
This commit should land before accept_encoding, because the feature
opens more possibilities to step into the problem that is fixed by this
patch.
Also I think the the fix should land to all 1.10+ branches: please,
verify that this will be done right when the patch will land.
The code LGTM. See several comments about texts below.
WBR, Alexander Turenko.
> http: remove redundant & incorrect case for curl_request code
Please, try to fit the commit header to 50 symbols if possible. Say
(just as example):
| httpc: fix assertion fail after a write error
Is I remember it right: any CURLE_WRITE_ERROR did lead to this assertion
fail? Or only unknown Content-Encoding?
>
> After executing curl request we need to process curl_request code. It
> might be CURLE_WRITE_ERROR. We had special case for it, which assumed
> diagnostic message being set and contained corresponding assert, though
> it is incorrect. Better way is to handle it as any other non-standard
> event.
I would add an information how do you catched this: your story about
unknown encoding. This woudl answer to the question why the patch was
made: I think it is good property, when a reader can understood it from
a commit message even when (s)he it is not much involved into a project.
>
> Part of #4232
> ---
> src/httpc.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/src/httpc.c b/src/httpc.c
> index 146a6f067..22b54d16a 100644
> --- a/src/httpc.c
> +++ b/src/httpc.c
> @@ -445,16 +445,12 @@ httpc_execute(struct httpc_request *req, double timeout)
> case CURLE_COULDNT_RESOLVE_PROXY:
> case CURLE_COULDNT_RESOLVE_HOST:
> case CURLE_COULDNT_CONNECT:
> + case CURLE_WRITE_ERROR:
> /* 595 Connection Problem (AnyEvent non-standard) */
> req->status = 595;
> req->reason = curl_easy_strerror(req->curl_request.code);
> ++env->stat.failed_requests;
> break;
> - case CURLE_WRITE_ERROR:
> - /* Diag is already set by curl_write_cb() */
> - assert(!diag_is_empty(&fiber()->diag));
> - ++env->stat.failed_requests;
> - return -1;
> case CURLE_OUT_OF_MEMORY:
> diag_set(OutOfMemory, 0, "curl", "internal");
> ++env->stat.failed_requests;
> --
> 2.17.1
>
More information about the Tarantool-patches
mailing list