[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
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
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
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); }
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;
Hi On Wed, Nov 25, 2020 at 10:20 PM Allan McRae <allan@archlinux.org> wrote:
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.
Hi On Wed, Nov 25, 2020 at 10:32 PM Allan McRae <allan@archlinux.org> wrote:
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; }
participants (3)
-
Allan McRae
-
Anatol Pomozov
-
morganamilo