On Thu, Aug 18, 2011 at 2:42 AM, Lukas Fleischer
<[email protected]> wrote:
> Use the STRDUP macro instead of strdup() for the sake of better error
> handling on memory allocation failures.
>
> Signed-off-by: Lukas Fleischer <[email protected]>
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
>
>
>
