[pacman-dev] [PATCH 1/2] lib/libalpm/dload.c: Use STRDUP() instead of strdup()

[Dan McGee]

On Thu, Aug 18, 2011 at 2:42 AM, Lukas Fleischer
<archlinux at 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 at 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
>
>
>