Skip to content
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

Preparation for http multi #219

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

elboulangero
Copy link
Contributor

This pull requests brings various rework to the the casync-http client. Mostly it's code factorization, renaming, etc... The aim is to have a better codebase for the big chunk of work coming: parallel http requests.

Signed-off-by: Arnaud Rebillout <[email protected]>
This is to prepare the next commit, where we will use the protocol enum
for more than just the protocol passed in arguments, and the arg_ prefix
won't make sense anymore.

Signed-off-by: Arnaud Rebillout <[email protected]>
This commits brings in two helpers:
- protocol_str() to convert an enum protocol to a string, which is
  useful mainly for logs.
- protocol_status_ok() as a unique place to check if the protocol status
  that we get from libcurl means OK or KO.

Signed-off-by: Arnaud Rebillout <[email protected]>
It seems to me that the condition PROCESS_UNTIL_WRITTEN is reached when
there's no more data to write, hence ca_remote_has_unwritten() returns
0.

And it also seems that this is could be copy/paste mistake, as all the
code above is similar, but the condition matches the function we call,
ie:
- PROCESS_UNTIL_CAN_PUT_CHUNK > ca_remote_can_put_chunk
- PROCESS_UNTIL_CAN_PUT_INDEX > ca_remote_can_put_index
- PROCESS_UNTIL_CAN_PUT_ARCHIVE > ca_remote_can_put_archive
- PROCESS_UNTIL_HAVE_REQUEST > ca_remote_has_pending_requests

But here, the function returns the opposite of what we want:
- PROCESS_UNTIL_WRITTEN > ca_remote_has_unwritten

Note that I didn't observe any bug due to that, and the test suite
succeeds before and after this patch.

Signed-off-by: Arnaud Rebillout <[email protected]>
While working on this code, I stumbled on cases where casync got stuck
because we forgot to call PROCESS_UNTIL_WRITTEN here. Well, TBH as long
as we download chunks, we're fine because we end up calling
PROCESS_UNTIL_WRITTEN afterwards.

But in any case, it seems more correct to sync after downloading these
files, and it doesn't hurt.

Signed-off-by: Arnaud Rebillout <[email protected]>
The way we use curl handle is that we create it once, and then re-use it
again and again, as it's more efficient than re-allocating a new one for
each request.

By looking at the code closely, it turns out that the setup of the curl
handle needs to be done only once, then afterwards we only need to
change the URL in order to re-use the handle.

So this commit brings two helper functions to reflect that:
- make_curl_easy_handle() does the init work and set all the options for
  the handle.
- configure_curl_easy_handle() does the things that are needed in order
  to re-use the handle. In effect, it only sets the URL.

Additionally, this commit introduces curl_easy_cleanupp, in order to use
automatic pointers.

Signed-off-by: Arnaud Rebillout <[email protected]>
These macros aim to make setting curl options easier.

CURL_SETOPT_EASY() sets the option, and on failure it outputs a generic
error message with the name of the option that failed, and returns -EIO.

The CURL_SETOPT_EASY_CANFAIL() variant does not return, it only outputs
an error message.

Signed-off-by: Arnaud Rebillout <[email protected]>
This removes the need for the 'finish' label, hence a bunch of goto go
away.

Signed-off-by: Arnaud Rebillout <[email protected]>
elboulangero and others added 4 commits June 25, 2019 16:59
The goal is to make the run() function more readable, and only outline
the major steps, while the bulk of the work is left to other functions.

Signed-off-by: Arnaud Rebillout <[email protected]>
This long option does not use a short option (see the optstring in call
of getopt_long).

Use a constant instead, as it is done in casync-tool.c.
This can be useful for testing, if ever we do HTTP2/SSL with a local,
untrusted server.

Signed-off-by: Arnaud Rebillout <[email protected]>
@elboulangero elboulangero force-pushed the mr/2019-06/prep-for-http-multi branch from a7dcd25 to bc85e26 Compare June 25, 2019 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants