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

[Allan McRae]

On 15/04/16 22:10, Silvan Jegen wrote:
> 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?
> 

Both adjusted.  Thanks for the review!

A