[pacman-dev] [PATCH] Handle bad .part files when encountered

[Silvan Jegen]

Heyho

Just two suggestions below.

On Fri, Apr 15, 2016 at 1:54 PM, Allan McRae <allan at archlinux.org> wrote:
> We can end up with .part files downloaded that are unable to be used by pacman
> in two ways:  1) a well timed ctrl+c right at the rename of the download file
> can leave a complete download with a .part extension, or 2) for some reason
> (bad mirror) a .part file can be larger than the file to be downloaded. In
> both cases, pacman fails with a very cryptic error message.
>
> When these files are encountered by libalpm, they are now either renamed to
> remove the if .part suffix if the download is complete, or deleted if the
> download file is too large.
>
> Fixes FS#35789 and FS#39257.
>
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---
>  lib/libalpm/sync.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 00b68d0..95e34f8 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -337,7 +337,27 @@ static int compute_download_size(alpm_pkg_t *newpkg)
>                         /* subtract the size of the .part file */
>                         _alpm_log(handle, ALPM_LOG_DEBUG, "using (package - .part) size\n");
>                         size = newpkg->size - st.st_size;
> -                       size = size < 0 ? 0 : size;
> +                       if(size < 0) {
> +                               /* we have a bad .part file - delete it */
> +                               _alpm_log(handle, ALPM_LOG_DEBUG, "deleting invalid .part file\n");
> +                               unlink(fpath);
> +                               size = newpkg->size;
> +                               goto finish;
> +                       } else if (size == 0) {
> +                               /* the .part file is complete, but needs renamed */

This comment should probably be:

/* the .part file is complete, but needs to be renamed */


> +                               char *newpath;
> +                               size_t fpathlen = strlen(fpath);
> +
> +                               _alpm_log(handle, ALPM_LOG_DEBUG, "renaming complete .part file\n");
> +
> +                               CALLOC(newpath, fpathlen + 1, sizeof(char), return -1);
> +                               strcpy(newpath, fpath);
> +                               newpath[fpathlen - 5] = '\0';

Maybe

newpath[fpathlen - strlen(".part")] = '\0';

would be clearer?


Cheers,

Silvan

> +                               rename(fpath, newpath);
> +                               FREE(newpath);
> +
> +                               goto finish;
> +                       }
>                 }
>
>                 /* tell the caller that we have a partial */
> --
> 2.7.4