Re: [pacman-dev] [PATCH] Cleanup the old sequential download code
Hi
On Wed, Jun 10, 2020 at 8:12 PM Allan McRae wrote:
>
> On 12/5/20 3:16 am, Anatol Pomozov wrote:
> > All users of _alpm_download() have been refactored to the new API.
> > It is time to remove the old _alpm_download() functionality now.
> >
> > This change also removes obsolete SIGPIPE signal handler functionality
> > (this is a leftover from libfetch days).
> >
> > Signed-off-by: Anatol Pomozov
> > ---
> > lib/libalpm/dload.c | 323 +---
> > lib/libalpm/dload.h | 4 -
> > 2 files changed, 3 insertions(+), 324 deletions(-)
> >
> > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> > index 43fe9847..4dbb011f 100644
> > --- a/lib/libalpm/dload.c
> > +++ b/lib/libalpm/dload.c
> > @@ -78,10 +78,6 @@ enum {
> > };
> >
> > static int dload_interrupted;
> > -static void inthandler(int UNUSED signum)
> > -{
> > - dload_interrupted = ABORT_SIGINT;
> > -}
> >
> > static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t
> > dlnow,
> > curl_off_t UNUSED ultotal, curl_off_t UNUSED ulnow)
> > @@ -236,8 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t
> > size, size_t nmemb, void *u
> > return realsize;
> > }
> >
> > -static void curl_set_handle_opts(struct dload_payload *payload,
> > - CURL *curl, char *error_buffer)
> > +static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
> > {
>
> This change seems beyond the scope for this patch. Can you split that
> and the other related changes below into a separate patch.
Inlining "error_buffer" is related to the "serial download" code cleanup.
The old (serial) codepath has been using a stack variable error_buffer
and was passing it to curl_set_handle_opts() function as a parameter.
With the concurrent download we cannot share a stack variable between
all payloads, instead we initialize a separate buffer for each
payload. Now "error_buffer is part of dload_payload{}.
With this serial codepath removed we don't have any local variable for
error_buffer. And we don't need to pass "error_buffer" to
curl_set_handle_opts() - "error_buffer" is already part of *payload
parameter.
This parameter inlining organically belongs to this refactoring IMO.
Re: [pacman-dev] [PATCH] Cleanup the old sequential download code
On 12/5/20 3:16 am, Anatol Pomozov wrote:
> All users of _alpm_download() have been refactored to the new API.
> It is time to remove the old _alpm_download() functionality now.
>
> This change also removes obsolete SIGPIPE signal handler functionality
> (this is a leftover from libfetch days).
>
> Signed-off-by: Anatol Pomozov
> ---
> lib/libalpm/dload.c | 323 +---
> lib/libalpm/dload.h | 4 -
> 2 files changed, 3 insertions(+), 324 deletions(-)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 43fe9847..4dbb011f 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -78,10 +78,6 @@ enum {
> };
>
> static int dload_interrupted;
> -static void inthandler(int UNUSED signum)
> -{
> - dload_interrupted = ABORT_SIGINT;
> -}
>
> static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t
> dlnow,
> curl_off_t UNUSED ultotal, curl_off_t UNUSED ulnow)
> @@ -236,8 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t
> size, size_t nmemb, void *u
> return realsize;
> }
>
> -static void curl_set_handle_opts(struct dload_payload *payload,
> - CURL *curl, char *error_buffer)
> +static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
> {
This change seems beyond the scope for this patch. Can you split that
and the other related changes below into a separate patch.
Otherwise fine.
A
Re: [pacman-dev] [PATCH] Cleanup the old sequential download code
On 14/5/20 6:03 pm, Anatol Pomozov wrote: > Given that in other patches we rename new functions to its old name it > makes sense to rename _alpm_multi_download to _alpm_download. That would make sense. A
Re: [pacman-dev] [PATCH] Cleanup the old sequential download code
Hi
On Mon, May 11, 2020 at 10:16 AM Anatol Pomozov
wrote:
>
> All users of _alpm_download() have been refactored to the new API.
> It is time to remove the old _alpm_download() functionality now.
>
> This change also removes obsolete SIGPIPE signal handler functionality
> (this is a leftover from libfetch days).
>
> Signed-off-by: Anatol Pomozov
> ---
> lib/libalpm/dload.c | 323 +---
> lib/libalpm/dload.h | 4 -
> 2 files changed, 3 insertions(+), 324 deletions(-)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 43fe9847..4dbb011f 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -78,10 +78,6 @@ enum {
> };
>
> static int dload_interrupted;
> -static void inthandler(int UNUSED signum)
> -{
> - dload_interrupted = ABORT_SIGINT;
> -}
>
> static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t
> dlnow,
> curl_off_t UNUSED ultotal, curl_off_t UNUSED ulnow)
> @@ -236,8 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t
> size, size_t nmemb, void *u
> return realsize;
> }
>
> -static void curl_set_handle_opts(struct dload_payload *payload,
> - CURL *curl, char *error_buffer)
> +static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
> {
> alpm_handle_t *handle = payload->handle;
> const char *useragent = getenv("HTTP_USER_AGENT");
> @@ -247,7 +242,7 @@ static void curl_set_handle_opts(struct dload_payload
> *payload,
> * to reset the handle's parameters for each time it's used. */
> curl_easy_reset(curl);
> curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
> - curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, error_buffer);
> + curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, payload->error_buffer);
> curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L);
> curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 10L);
> curl_easy_setopt(curl, CURLOPT_FILETIME, 1L);
> @@ -301,24 +296,6 @@ static void curl_set_handle_opts(struct dload_payload
> *payload,
> }
> }
>
> -static void mask_signal(int signum, void (*handler)(int),
> - struct sigaction *origaction)
> -{
> - struct sigaction newaction;
> -
> - newaction.sa_handler = handler;
> - sigemptyset(_mask);
> - newaction.sa_flags = 0;
> -
> - sigaction(signum, NULL, origaction);
> - sigaction(signum, , NULL);
> -}
> -
> -static void unmask_signal(int signum, struct sigaction *sa)
> -{
> - sigaction(signum, sa, NULL);
> -}
> -
> static FILE *create_tempfile(struct dload_payload *payload, const char
> *localpath)
> {
> int fd;
> @@ -353,259 +330,6 @@ static FILE *create_tempfile(struct dload_payload
> *payload, const char *localpat
> /* RFC1123 states applications should support this length */
> #define HOSTNAME_SIZE 256
>
> -static int curl_download_internal(struct dload_payload *payload,
> - const char *localpath, char **final_file, const char
> **final_url)
> -{
> - int ret = -1;
> - FILE *localf = NULL;
> - char *effective_url;
> - char hostname[HOSTNAME_SIZE];
> - char error_buffer[CURL_ERROR_SIZE] = {0};
> - struct stat st;
> - long timecond, remote_time = -1;
> - double remote_size, bytes_dl;
> - struct sigaction orig_sig_pipe, orig_sig_int;
> - CURLcode curlerr;
> - alpm_download_event_init_t init_cb_data = {0};
> - alpm_download_event_completed_t completed_cb_data = {0};
> - /* shortcut to our handle within the payload */
> - alpm_handle_t *handle = payload->handle;
> - CURL *curl = curl_easy_init();
> - handle->pm_errno = ALPM_ERR_OK;
> - payload->curl = curl;
> -
> - /* make sure these are NULL */
> - FREE(payload->tempfile_name);
> - FREE(payload->destfile_name);
> - FREE(payload->content_disp_name);
> -
> - payload->tempfile_openmode = "wb";
> - if(!payload->remote_name) {
> - STRDUP(payload->remote_name, get_filename(payload->fileurl),
> - RET_ERR(handle, ALPM_ERR_MEMORY, -1));
> - }
> - if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
> - _alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"),
> payload->fileurl);
> - RET_ERR(handle, ALPM_ERR_SERVER_BAD_URL, -1);
> - }
> -
> - if(payload->remote_name && strlen(payload->remote_name) > 0 &&
> - strcmp(payload->remote_name, ".sig") != 0) {
> - payload->destfile_name = get_fullpath(localpath,
> payload->remote_name, "");
> - payload->tempfile_name = get_fullpath(localpath,
> payload->remote_name, ".part");
> - if(!payload->destfile_name || !payload->tempfile_name) {
> - goto cleanup;
> - }
> - } else {
> -
[pacman-dev] [PATCH] Cleanup the old sequential download code
All users of _alpm_download() have been refactored to the new API.
It is time to remove the old _alpm_download() functionality now.
This change also removes obsolete SIGPIPE signal handler functionality
(this is a leftover from libfetch days).
Signed-off-by: Anatol Pomozov
---
lib/libalpm/dload.c | 323 +---
lib/libalpm/dload.h | 4 -
2 files changed, 3 insertions(+), 324 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 43fe9847..4dbb011f 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -78,10 +78,6 @@ enum {
};
static int dload_interrupted;
-static void inthandler(int UNUSED signum)
-{
- dload_interrupted = ABORT_SIGINT;
-}
static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t dlnow,
curl_off_t UNUSED ultotal, curl_off_t UNUSED ulnow)
@@ -236,8 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t size,
size_t nmemb, void *u
return realsize;
}
-static void curl_set_handle_opts(struct dload_payload *payload,
- CURL *curl, char *error_buffer)
+static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
{
alpm_handle_t *handle = payload->handle;
const char *useragent = getenv("HTTP_USER_AGENT");
@@ -247,7 +242,7 @@ static void curl_set_handle_opts(struct dload_payload
*payload,
* to reset the handle's parameters for each time it's used. */
curl_easy_reset(curl);
curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
- curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, error_buffer);
+ curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, payload->error_buffer);
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L);
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 10L);
curl_easy_setopt(curl, CURLOPT_FILETIME, 1L);
@@ -301,24 +296,6 @@ static void curl_set_handle_opts(struct dload_payload
*payload,
}
}
-static void mask_signal(int signum, void (*handler)(int),
- struct sigaction *origaction)
-{
- struct sigaction newaction;
-
- newaction.sa_handler = handler;
- sigemptyset(_mask);
- newaction.sa_flags = 0;
-
- sigaction(signum, NULL, origaction);
- sigaction(signum, , NULL);
-}
-
-static void unmask_signal(int signum, struct sigaction *sa)
-{
- sigaction(signum, sa, NULL);
-}
-
static FILE *create_tempfile(struct dload_payload *payload, const char
*localpath)
{
int fd;
@@ -353,259 +330,6 @@ static FILE *create_tempfile(struct dload_payload
*payload, const char *localpat
/* RFC1123 states applications should support this length */
#define HOSTNAME_SIZE 256
-static int curl_download_internal(struct dload_payload *payload,
- const char *localpath, char **final_file, const char
**final_url)
-{
- int ret = -1;
- FILE *localf = NULL;
- char *effective_url;
- char hostname[HOSTNAME_SIZE];
- char error_buffer[CURL_ERROR_SIZE] = {0};
- struct stat st;
- long timecond, remote_time = -1;
- double remote_size, bytes_dl;
- struct sigaction orig_sig_pipe, orig_sig_int;
- CURLcode curlerr;
- alpm_download_event_init_t init_cb_data = {0};
- alpm_download_event_completed_t completed_cb_data = {0};
- /* shortcut to our handle within the payload */
- alpm_handle_t *handle = payload->handle;
- CURL *curl = curl_easy_init();
- handle->pm_errno = ALPM_ERR_OK;
- payload->curl = curl;
-
- /* make sure these are NULL */
- FREE(payload->tempfile_name);
- FREE(payload->destfile_name);
- FREE(payload->content_disp_name);
-
- payload->tempfile_openmode = "wb";
- if(!payload->remote_name) {
- STRDUP(payload->remote_name, get_filename(payload->fileurl),
- RET_ERR(handle, ALPM_ERR_MEMORY, -1));
- }
- if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
- _alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"),
payload->fileurl);
- RET_ERR(handle, ALPM_ERR_SERVER_BAD_URL, -1);
- }
-
- if(payload->remote_name && strlen(payload->remote_name) > 0 &&
- strcmp(payload->remote_name, ".sig") != 0) {
- payload->destfile_name = get_fullpath(localpath,
payload->remote_name, "");
- payload->tempfile_name = get_fullpath(localpath,
payload->remote_name, ".part");
- if(!payload->destfile_name || !payload->tempfile_name) {
- goto cleanup;
- }
- } else {
- /* URL doesn't contain a filename, so make a tempfile. We can't
support
-* resuming this kind of download; partial transfers will be
destroyed */
- payload->unlink_on_fail = 1;
-
- localf = create_tempfile(payload, localpath);
- if(localf ==
