Re: [Bug-wget] [GSOC] Bugfixes

[Giuseppe Scrivano]

Hi Hubert,

Hubert Tarasiuk <address@hidden> writes:

> Now I would like to work on eliminating the other label from gethttp
> (used for http authentication). I was thinking about eventually moving
> this logic to http_loop (the socket is closed in this case anyway). Ie.
> on HTTP_STATUS_UNAUTHORIZED, the function would return to http_loop, and
> appropriate actions would be taken in that function (ie. another call to
> gethttp, with modified arguments).
>
> Later on, the code handling each http status code could be perhaps
> isolated and separated into smaller functions.
>
> Am I moving in the right directions? Do you have any suggestions?

Good work!  Yes, it looks like going in the right direction.


> And BTW what do you think about initializing some pointer variables
> (like `head` or `resp`) to NULL in order to simplify resources management?

Actually I have suggested something similar on this mailing list few
days ago.  I think we should move to have a single exit point in gethttp
where we take care of all the cleanup.  This will require to initialize
all the pointers to NULL and replace any "return" with a "goto
cleanup" instead of having several xfree called all over the function.

Few comments:

> From a1c00b354b7952dd35a7f5e26232b4d273b921eb Mon Sep 17 00:00:00 2001
> From: hutabert <address@hidden>

please configure your ~/.gitconfig to contain your real name, it will be
used by git commit to set the proper author name.

Mine, for example, looks like:

$ cat ~/.gitconfig 
[user]
        name = Giuseppe Scrivano
        email = address@hidden

> Date: Fri, 27 Mar 2015 14:00:33 +0100
> Subject: [PATCH 1/2] Factor out set_content_type function from gethttp
>
> ---
>  src/http.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/src/http.c b/src/http.c
> index 53c9818..ff7f58f 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -2330,6 +2330,27 @@ open_output_stream (struct http_stat *hs, int count, 
> FILE **fp)
>        return RETROK;
>  }
>  
> +/* Set proper type flags based on type string. */

minor issue, but will be useful in the long term, we leave two spaes
after a dot :)

/* Set proper type flags based on type string.  */

Both patches miss a ChangeLog style commit message.  We require it as
part of following the GNU coding standards[1].  We do not modify
directly the ChangeLog file, as described there, but we generate it
starting from the git commit logs, please take a look at some commits
done last month to see how it is done.

Thanks for working on these issues!

Giuseppe

1) https://www.gnu.org/prep/standards/