[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget
From: |
Yousong Zhou |
Subject: |
Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget |
Date: |
Mon, 31 Mar 2014 10:01:58 +0800 |
Hi,
On 28 March 2014 20:33, Jure Grabnar <address@hidden> wrote:
> Hi,
>
> Thank you Yousong. I've listened to your advice and changed type of
> resource->type to
> enum url_scheme. Now it looks much cleaner.
Using enum is a step forward.
> @@ -134,7 +135,20 @@ parse_metalink(char *input_file)
> ++(file->num_of_res);
>
> resource->url = xstrdup ((*resources)->url);
> - resource->type = ((*resources)->type ? xstrdup
> ((*resources)->type) : NULL);
> +
> + if ((*resources)->type)
> + {
> + /* Append "://" to resource type so url_scheme() recognizes
> type */
> + char *temp_url = malloc ( strlen ( (*resources)->type) + 4);
> + sprintf (temp_url, "%s://", (*resources)->type);
> +
> + resource->type = url_scheme (temp_url);
> +
> + free (temp_url);
> + }
This is a little hacky. Adding a utility function like
url_scheme_str_to_enum() will be better.
> + else
> + resource->type = url_scheme (resource->url);
> +
> resource->location = ((*resources)->location ? xstrdup
> ((*resources)->location) : NULL);
> resource->preference = (*resources)->preference;
> resource->maxconnections = (*resources)->maxconnections;
> @@ -143,7 +157,7 @@ parse_metalink(char *input_file)
> (file->resources) = resource;
> }
>
> - for (checksums = (*files)->checksums; *checksums; ++checksums)
> + for (checksums = (*files)->checksums; checksums && *checksums;
> ++checksums)
Good catch. Should do the same NULL check for (*files)->resources.
> {
> mlink_checksum *checksum = malloc (sizeof(mlink_checksum));
>
>
<...>
> @@ -215,19 +229,25 @@ elect_resources (mlink *mlink)
>
> while (res_next = res->next)
> {
> - if (strcmp(res_next->type, "ftp") && strcmp(res_next->type,
> "http"))
> + if (schemes_are_similar_p (res_next->type, SCHEME_INVALID))
> {
> res->next = res_next->next;
> free(res_next);
> +
> + --(file->num_of_res);
> }
> else
> res = res_next;
> }
> res = file->resources;
> - if (strcmp(res->type, "ftp") && strcmp(res->type, "http"))
> + if (schemes_are_similar_p (res->type, SCHEME_INVALID))
> {
> file->resources = res->next;
If I am right, this will set it to NULL if file->num_of_res is 1.
> - free(res);
> + free (res);
> +
> + --(file->num_of_res);
> + if (!file->num_of_res)
> + file->resources = NULL;
So explicitly setting it to NULL is not needed.
> }
> }
> }
>
>
> I also added check for whenever there's no resources available to download a
> file.
>
> Second patch remains unchanged.
>
> Regards,
>
>
> Jure Grabnar
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, (continued)
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Darshit Shah, 2014/03/20
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Jure Grabnar, 2014/03/20
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Darshit Shah, 2014/03/21
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Yousong Zhou, 2014/03/21
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Jure Grabnar, 2014/03/22
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Yousong Zhou, 2014/03/22
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Darshit Shah, 2014/03/27
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Jure Grabnar, 2014/03/27
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Jure Grabnar, 2014/03/28
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Jure Grabnar, 2014/03/28
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget,
Yousong Zhou <=
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Jure Grabnar, 2014/03/31
- Re: [Bug-wget] Fwd: [GSoC] Extend concurrency support in Wget, Yousong Zhou, 2014/03/31