From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 83F00430D56 for ; Tue, 29 Oct 2019 02:42:50 +0300 (MSK) Date: Tue, 29 Oct 2019 02:42:41 +0300 From: Alexander Turenko Message-ID: <20191028234241.bzttdqr4cl3zuxl6@tkn_work_nb> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v4 2/4] http: remove redundant & incorrect case for curl_request code List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ilya Kosarev Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org 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 >