Re: [Bug-wget] [PATCH 19/25] Bugfix: Prevent sorting of unallocated metalink resource/metaurl

[Giuseppe Scrivano]

Matthew White <address@hidden> writes:

> From cbed79d8394c8c0a74b2f65cb92d01178184e5fe Mon Sep 17 00:00:00 2001
> From: Matthew White <address@hidden>
> Date: Thu, 25 Aug 2016 15:26:18 +0200
> Subject: [PATCH 19/25] Bugfix: Prevent sorting of unallocated metalink
>  resource/metaurl
>
> * src/main.c (main): Sort metalink resources only if there is more
>   than 1 real resource allocated
> * src/http.c (metalink_from_http): Sort metalink resources only if
>   there is more than 1 real resource allocated
> * src/http.c (metalink_from_http): Sort metalink metaurls only if
>   there is more than 1 real metaurl allocated
>
> Trying to sort the resources/metaurls of a metalink when there is only
> one element allocated and no tail, or no elements at all, will lead to
> a segmentation fault.
>
> Also, sorting the resources/metaurls of a metalink when there is only
> one element allocated and a NULL tail is useless.
> ---
>  src/http.c | 6 ++++--
>  src/main.c | 9 +++++----
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/http.c b/src/http.c
> index e6af7c1..a7ad76e 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -2933,8 +2933,10 @@ metalink_from_http (const struct response *resp, const 
> struct http_stat *hs,
>  
>    /* Metalink data is OK. Now we just need to sort the resources based
>       on their priorities, preference, and perhaps location.  */
> -  stable_sort (mfile->resources, res_count, sizeof (metalink_resource_t *), 
> metalink_res_cmp);
> -  stable_sort (mfile->metaurls, meta_count, sizeof (metalink_metaurl_t *), 
> metalink_meta_cmp);
> +  if (res_count > 1)
> +    stable_sort (mfile->resources, res_count, sizeof (metalink_resource_t 
> *), metalink_res_cmp);
> +  if (meta_count > 1)
> +    stable_sort (mfile->metaurls, meta_count, sizeof (metalink_metaurl_t *), 
> metalink_meta_cmp);
>  
>    /* Restore sensible preference values (in case someone cares to look).  */
>    for (i = 0; i < res_count; ++i)
> diff --git a/src/main.c b/src/main.c
> index ac6ee2c..11ea86d 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -2136,10 +2136,11 @@ only if outputting to a regular file.\n"));
>                    for (mres_ptr = mfile->resources; *mres_ptr; mres_ptr++)
>                      mres_count++;
>  
> -                  stable_sort (mfile->resources,
> -                               mres_count,
> -                               sizeof (metalink_resource_t *),
> -                               metalink_res_cmp);
> +                  if (mres_count > 1)
> +                    stable_sort (mfile->resources,
> +                                 mres_count,
> +                                 sizeof (metalink_resource_t *),
> +                                 metalink_res_cmp);
>                  }
>              }
>            retr_err = retrieve_from_metalink (metalink);

stable_sort is already checking for size > 1.  Isn't this redundant?

Regards,
Giuseppe