On Thu, Aug 18, 2011 at 2:42 AM, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
Use the STRDUP macro instead of strdup() for the sake of better error handling on memory allocation failures.
Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
Thanks- wrapped at 80 chars though.
--- Should we do some cleanup and free() payload if STRDUP() fails in alpm_fetch_pkgurl()?
Even in this case, I'm not sure the immediate RET_ERR() is the best thing to do. Instead you should probably set pm_errno and make sure ret = -1, but not return right away so that the other cleanup logic takes place (notably the signal handler restoration, didn't realize that was so late). In fetch_pkgurl(), you may want to add an error: label that is jumped to instead of the RET_ERR() calls directly, and add the necessary cleanup logic there, or something along those lines to ensure things are unwound correctly. However, for the most part, we assume a memory allocation failure is not something transient and we will eventually die.
lib/libalpm/dload.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 68a68e9..a1b6364 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -387,10 +387,10 @@ cleanup: tempfile, destfile, strerror(errno)); ret = -1; } else if(final_file) { - *final_file = strdup(strrchr(destfile, '/') + 1); + STRDUP(*final_file, strrchr(destfile, '/') + 1, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); } } else if(final_file) { - *final_file = strdup(strrchr(tempfile, '/') + 1); + STRDUP(*final_file, strrchr(tempfile, '/') + 1, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); } }
@@ -456,7 +456,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url)
CALLOC(payload, 1, sizeof(*payload), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); payload->handle = handle; - payload->fileurl = strdup(url); + STRDUP(payload->fileurl, url, RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); payload->allow_resume = 1;
/* download the file */ -- 1.7.6