[pacman-dev] [PATCH 1/5] libalpm: set parallel_downloads to 1 when creating the handle
Fixes FS#68729 diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 34c5b4b2..5d36136d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -61,6 +61,8 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, /* set default database extension */ STRDUP(myhandle->dbext, ".db", goto nomem); + myhandle->parallel_downloads = 1; + lockfilelen = strlen(myhandle->dbpath) + strlen(lf) + 1; MALLOC(myhandle->lockfile, lockfilelen, goto nomem); snprintf(myhandle->lockfile, lockfilelen, "%s%s", myhandle->dbpath, lf); -- 2.29.2
Fixes FS#68728: diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 673e769f..d43e6d45 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -586,7 +586,7 @@ cleanup: unlink(payload->tempfile_name); } - if(!payload->signature) { + if(handle->dlcb && !payload->signature) { alpm_download_event_completed_t cb_data = {0}; cb_data.total = bytes_dl; cb_data.result = ret; @@ -719,7 +719,7 @@ static int curl_download_internal(alpm_handle_t *handle, struct dload_payload *payload = payloads->data; if(curl_add_payload(handle, curlm, payload, localpath) == 0) { - if(!payload->signature) { + if(handle->dlcb && !payload->signature) { alpm_download_event_init_t cb_data = {.optional = payload->errors_ok}; handle->dlcb(payload->remote_name, ALPM_DOWNLOAD_INIT, &cb_data); } -- 2.29.2
On 24/11/20 10:39 pm, morganamilo wrote:
Fixes FS#68728:
Thanks - I had a quick skim elsewhere and looks like this covers the issue fully.
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 673e769f..d43e6d45 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -586,7 +586,7 @@ cleanup: unlink(payload->tempfile_name); }
- if(!payload->signature) { + if(handle->dlcb && !payload->signature) { alpm_download_event_completed_t cb_data = {0}; cb_data.total = bytes_dl; cb_data.result = ret; @@ -719,7 +719,7 @@ static int curl_download_internal(alpm_handle_t *handle, struct dload_payload *payload = payloads->data;
if(curl_add_payload(handle, curlm, payload, localpath) == 0) { - if(!payload->signature) { + if(handle->dlcb && !payload->signature) { alpm_download_event_init_t cb_data = {.optional = payload->errors_ok}; handle->dlcb(payload->remote_name, ALPM_DOWNLOAD_INIT, &cb_data); }
Hi
On Wed, Nov 25, 2020 at 10:32 PM Allan McRae
On 24/11/20 10:39 pm, morganamilo wrote:
Fixes FS#68728:
Thanks - I had a quick skim elsewhere and looks like this covers the issue fully.
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 673e769f..d43e6d45 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -586,7 +586,7 @@ cleanup: unlink(payload->tempfile_name); }
- if(!payload->signature) { + if(handle->dlcb && !payload->signature) { alpm_download_event_completed_t cb_data = {0}; cb_data.total = bytes_dl; cb_data.result = ret; @@ -719,7 +719,7 @@ static int curl_download_internal(alpm_handle_t *handle, struct dload_payload *payload = payloads->data;
if(curl_add_payload(handle, curlm, payload, localpath) == 0) { - if(!payload->signature) { + if(handle->dlcb && !payload->signature) { alpm_download_event_init_t cb_data = {.optional = payload->errors_ok}; handle->dlcb(payload->remote_name, ALPM_DOWNLOAD_INIT, &cb_data); }
The patch LGTM. And for the record, the third place where we use handle->dlcb (lib/libalpm/dload.c around line 137) already has a guard: if(payload->handle->dlcb == NULL) { return 0; }
The comment makes it seem that the result itself is an error code. But all it does is simply return -1 to indicate an error occured; diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 614a530c..6a7323e0 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -728,9 +728,9 @@ typedef struct { /* total bytes in file */ off_t total; /* download result code: - * 0 - download completed successfully - * 1 - the file is up-to-date - * negative - error code + * 0 - download completed successfully + * 1 - the file is up-to-date + * -1 - error */ int result; } alpm_download_event_completed_t; -- 2.29.2
On 24/11/20 10:39 pm, morganamilo wrote:
The comment makes it seem that the result itself is an error code. But all it does is simply return -1 to indicate an error occured;
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 614a530c..6a7323e0 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -728,9 +728,9 @@ typedef struct { /* total bytes in file */ off_t total; /* download result code: - * 0 - download completed successfully - * 1 - the file is up-to-date - * negative - error code + * 0 - download completed successfully + * 1 - the file is up-to-date + * -1 - error
I have a suspicion this was supposed to indicate "<0" as a failure, but given we only use -1, this is fine.
*/ int result; } alpm_download_event_completed_t;
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 1310601a..16d4beaa 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -857,9 +857,7 @@ int SYMEXPORT alpm_option_set_parallel_downloads(alpm_handle_t *handle, { CHECK_HANDLE(handle, return -1); #ifdef HAVE_LIBCURL - if(num_streams < 1) { - return -1; - } + ASSERT(num_streams >= 1, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); handle->parallel_downloads = num_streams; #else (void)num_streams; /* silence unused variable warnings */ -- 2.29.2
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6a7323e0..50c4bb6b 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -934,6 +934,8 @@ int alpm_option_set_remote_file_siglevel(alpm_handle_t *handle, int level); int alpm_option_set_disable_dl_timeout(alpm_handle_t *handle, unsigned short disable_dl_timeout); +int alpm_option_get_parallel_downloads(alpm_handle_t *handle); + /** Sets number of parallel streams to download database and package files. * If the function is not called then the default value of '1' stream * (i.e. sequential download) is used. diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 16d4beaa..3994f314 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -300,6 +300,16 @@ const char SYMEXPORT *alpm_option_get_dbext(alpm_handle_t *handle) return handle->dbext; } +int SYMEXPORT alpm_option_get_parallel_downloads(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return -1); +#ifdef HAVE_LIBCURL + return handle->parallel_downloads; +#else + return 1; +#endif +} + int SYMEXPORT alpm_option_set_logcb(alpm_handle_t *handle, alpm_cb_log cb) { CHECK_HANDLE(handle, return -1); -- 2.29.2
On 24/11/20 10:39 pm, morganamilo wrote:
Fixes FS#68729
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 34c5b4b2..5d36136d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -61,6 +61,8 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, /* set default database extension */ STRDUP(myhandle->dbext, ".db", goto nomem);
+ myhandle->parallel_downloads = 1; + lockfilelen = strlen(myhandle->dbpath) + strlen(lf) + 1; MALLOC(myhandle->lockfile, lockfilelen, goto nomem); snprintf(myhandle->lockfile, lockfilelen, "%s%s", myhandle->dbpath, lf);
Thanks - I moved that line right under the curl bit to keep download stuff together. Allan
Hi
On Wed, Nov 25, 2020 at 10:20 PM Allan McRae
On 24/11/20 10:39 pm, morganamilo wrote:
Fixes FS#68729
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index 34c5b4b2..5d36136d 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -61,6 +61,8 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, /* set default database extension */ STRDUP(myhandle->dbext, ".db", goto nomem);
+ myhandle->parallel_downloads = 1; + lockfilelen = strlen(myhandle->dbpath) + strlen(lf) + 1; MALLOC(myhandle->lockfile, lockfilelen, goto nomem); snprintf(myhandle->lockfile, lockfilelen, "%s%s", myhandle->dbpath, lf);
Thanks - I moved that line right under the curl bit to keep download stuff together.
Thank you. The patch LGTM. The move to the curl section sounds good.
participants (3)
-
Allan McRae
-
Anatol Pomozov
-
morganamilo