On Wed, Jan 8, 2020, 07:30 Allan McRae <allan@archlinux.org> wrote:
Sync databases (and signatures) are downloaded into a temporary directory. On success, they are copied into the sync directory.
Why not use a temporary file in the sync dir? That allows you to rename atomically instead of copy+unlink. My guess is it's less code, too. Currently, this achieves nothing but adding complexity, but it does open
up the possibility of validating sync databases on download before replacing the old version.
Signed-off-by: Allan McRae <allan@archlinux.org> ---
This patch was long enough, and there is still quite a bit to do to validate the download sync databases before replacing the old ones, so best to stop here.
lib/libalpm/be_sync.c | 90 +++++++++++++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 21 deletions(-)
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index f1caddd8..99bdba54 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -173,8 +173,9 @@ valid: */ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) { - char *syncpath; - const char *dbext; + char *syncpath, *sigpath = NULL; + char *tmpdir = NULL, *tmppath = NULL, *tmpsig = NULL; + const char *dbpath, *dbext; alpm_list_t *i; int updated = 0; int ret = -1; @@ -193,11 +194,22 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) return 0; }
+ /* attempt to grab a lock */ + if(_alpm_handle_lock(handle)) { + RET_ERR(handle, ALPM_ERR_HANDLE_LOCK, -1); + } + syncpath = get_sync_dir(handle); if(!syncpath) { return -1; }
+ /* create temporary directory to download databases into */ + if(_alpm_mkdtemp(handle, &tmpdir) == 0) { + free(syncpath); + return -1; + } + /* force update of invalid databases to fix potential mismatched database/signature */ if(db->status & DB_STATUS_INVALID) { force = 1; @@ -207,15 +219,41 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) oldmask = umask(0022);
siglevel = alpm_db_get_siglevel(db); + dbext = handle->dbext;
- /* attempt to grab a lock */ - if(_alpm_handle_lock(handle)) { - free(syncpath); - umask(oldmask); - RET_ERR(handle, ALPM_ERR_HANDLE_LOCK, -1); + dbpath = _alpm_db_path(db); + if(dbpath == NULL) { + /* pm_errno set in _alpm_db_path() */ + ret = -1; + goto cleanup; }
- dbext = handle->dbext; + if((sigpath = _alpm_sigpath(handle, _alpm_db_path(db))) == NULL) { + /* pm_errno is set by _alpm_sigpath */ + ret = -1; + goto cleanup; + } + + if(asprintf(&tmppath, "%s%s%s", tmpdir, db->treename, dbext) == -1) { + handle->pm_errno = ALPM_ERR_MEMORY; + ret = -1; + goto cleanup; + } + + if(asprintf(&tmpsig, "%s%s%s.sig", tmpdir, db->treename, dbext) == -1) { + handle->pm_errno = ALPM_ERR_MEMORY; + ret = -1; + goto cleanup; + } + + /* copy current db into tempdir to allow up-to-date db handling */ + if(force == 0) { + if(_alpm_copyfile_with_time(dbpath, tmppath) != 0) { + handle->pm_errno = ALPM_ERR_SYSTEM; + ret = -1; + goto cleanup; + }; + }
for(i = db->servers; i; i = i->next) { const char *server = i->data, *final_db_url = NULL; @@ -240,22 +278,11 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) payload.force = force; payload.unlink_on_fail = 1;
- ret = _alpm_download(&payload, syncpath, NULL, &final_db_url); + ret = _alpm_download(&payload, tmpdir, NULL, &final_db_url); _alpm_dload_payload_reset(&payload); updated = (updated || ret == 0);
if(ret != -1 && updated && (siglevel & ALPM_SIG_DATABASE)) { - /* an existing sig file is no good at this point */ - char *sigpath = _alpm_sigpath(handle, _alpm_db_path(db)); - if(!sigpath) { - /* pm_errno is set by _alpm_sigpath */ - ret = -1; - goto cleanup; - } - unlink(sigpath); - free(sigpath); - - /* check if the final URL from internal downloader looks reasonable */ if(final_db_url != NULL) { if(strlen(final_db_url) < 3 @@ -295,7 +322,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) /* set hard upper limit of 16KiB */ payload.max_size = 16 * 1024;
- sig_ret = _alpm_download(&payload, syncpath, NULL, NULL); + sig_ret = _alpm_download(&payload, tmpdir, NULL, NULL); /* errors_ok suppresses error messages, but not the return code */ sig_ret = payload.errors_ok ? 0 : sig_ret; _alpm_dload_payload_reset(&payload); @@ -306,6 +333,15 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) } }
+ /* copy the downloaded database into the sync directory */ + unlink(dbpath); + _alpm_copyfile_with_time(tmppath, dbpath); + + unlink(sigpath); + if(_alpm_access(handle, NULL, sigpath, R_OK) == 0) { + _alpm_copyfile_with_time(tmpsig, sigpath); + } + cleanup: if(updated) { /* Cache needs to be rebuilt */ @@ -332,8 +368,20 @@ cleanup: handle->pm_errno = ALPM_ERR_OK; }
+ /* clean-up temporary directory */ + unlink(tmppath); + unlink(tmpsig); + if(rmdir(tmpdir)) { + _alpm_log(handle, ALPM_LOG_WARNING, + _("could not remove tmpdir %s\n"), tmpdir); + } + free(tmpdir); + free(tmppath); + free(tmpsig); + _alpm_handle_unlock(handle); free(syncpath); + free(sigpath); umask(oldmask); return ret; } -- 2.24.1