[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [GSOC] Bugfixes
From: |
Giuseppe Scrivano |
Subject: |
Re: [Bug-wget] [GSOC] Bugfixes |
Date: |
Fri, 27 Mar 2015 14:49:31 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
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/
- [Bug-wget] [GSOC] Bugfixes, Hubert Tarasiuk, 2015/03/25
- Re: [Bug-wget] [GSOC] Bugfixes, Giuseppe Scrivano, 2015/03/25
- Message not available
- Message not available
- Re: [Bug-wget] [GSOC] Bugfixes, Hubert Tarasiuk, 2015/03/25
- Re: [Bug-wget] [GSOC] Bugfixes, Hubert Tarasiuk, 2015/03/27
- Re: [Bug-wget] [GSOC] Bugfixes,
Giuseppe Scrivano <=
- Re: [Bug-wget] [GSOC] Bugfixes, Hubert Tarasiuk, 2015/03/27
- Re: [Bug-wget] [GSOC] Bugfixes, Hubert Tarasiuk, 2015/03/27
- Re: [Bug-wget] [GSOC] Bugfixes, Darshit Shah, 2015/03/28
- Re: [Bug-wget] [GSOC] Bugfixes, Hubert Tarasiuk, 2015/03/28