[pacman-dev] [PATCH] libalpm, fix leak if malloc fails in be_sync
--- lib/libalpm/be_sync.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index b09b060..9229622 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -178,6 +178,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) const char *dbext; alpm_list_t *i; int ret = -1; + int memory_error = 0; mode_t oldmask; alpm_handle_t *handle; alpm_siglevel_t level; @@ -230,8 +231,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) /* print server + filename into a buffer */ len = strlen(server) + strlen(db->treename) + strlen(dbext) + 2; - /* TODO fix leak syncpath and umask unset */ - MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + MALLOC(payload.fileurl, len, memory_error = 1; goto cleanup); snprintf(payload.fileurl, len, "%s/%s%s", server, db->treename, dbext); payload.handle = handle; payload.force = force; @@ -269,8 +269,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) len = strlen(server) + strlen(db->treename) + strlen(dbext) + 6; } - /* TODO fix leak syncpath and umask unset */ - MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + MALLOC(payload.fileurl, len, memory_error = 1; goto cleanup); if(final_db_url != NULL) { snprintf(payload.fileurl, len, "%s.sig", final_db_url); @@ -322,9 +321,10 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) } cleanup: - _alpm_handle_unlock(handle); free(syncpath); umask(oldmask); + ASSERT(memory_error == 0, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + _alpm_handle_unlock(handle); return ret; } -- 2.6.2
On 11/11/15 05:52, Rikard Falkeborn wrote:
--- lib/libalpm/be_sync.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
This needs reworked due to other changes made to that file. But I seem to recall Andrew had a comment on this... Andrew - did I recall correctly?
On 01/04/16 at 02:54pm, Allan McRae wrote:
On 11/11/15 05:52, Rikard Falkeborn wrote:
--- lib/libalpm/be_sync.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
This needs reworked due to other changes made to that file. But I seem to recall Andrew had a comment on this...
Andrew - did I recall correctly?
I think my only real issue with the patch itself was that it moved _alpm_handle_unlock when it shouldn't have. At this point this patch should just remove RET_ERR, manually set pm_errno, and break or continue the loop. apg
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Rikard Falkeborn