W dniu 27.03.2015 o 19:49, Giuseppe Scrivano pisze: > Few comments: > > please configure your ~/.gitconfig to contain your real name, it will be > used by git commit to set the proper author name. > > minor issue, but will be useful in the long term, we leave two spaes > after a dot :) > > 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. > I am attaching corrected patches. I additionally corrected few trailing whitespace errors that were present in the previous version.
>> 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. I see. I will take a closer look at this issue. Thank you for the hints. Hubert
From 7b7a50e38f3b196571ab2c243f65b4fcc62297a9 Mon Sep 17 00:00:00 2001 From: Hubert Tarasiuk <[email protected]> Date: Fri, 27 Mar 2015 14:00:33 +0100 Subject: [PATCH 1/2] Factor out set_content_type function from gethttp * src/http.c (gethttp): Move some code in... (set_content_type): ... a new function. --- src/http.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/http.c b/src/http.c index 53c9818..a127733 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. */ +static void +set_content_type (int *dt, const char *type) +{ + /* If content-type is not given, assume text/html. This is because + o f the multitud*e of broken CGI's that "forget" to generate the + content-type. */ + if (!type || + 0 == strncasecmp (type, TEXTHTML_S, strlen (TEXTHTML_S)) || + 0 == strncasecmp (type, TEXTXHTML_S, strlen (TEXTXHTML_S))) + *dt |= TEXTHTML; + else + *dt &= ~TEXTHTML; + + if (type && + 0 == strncasecmp (type, TEXTCSS_S, strlen (TEXTCSS_S))) + *dt |= TEXTCSS; + else + *dt &= ~TEXTCSS; +} + /* Retrieve a document through HTTP protocol. It recognizes status code, and correctly handles redirections. It closes the network socket. If it receives an error from the functions below it, it @@ -2957,21 +2978,7 @@ read_header: } } - /* If content-type is not given, assume text/html. This is because - of the multitude of broken CGI's that "forget" to generate the - content-type. */ - if (!type || - 0 == strncasecmp (type, TEXTHTML_S, strlen (TEXTHTML_S)) || - 0 == strncasecmp (type, TEXTXHTML_S, strlen (TEXTXHTML_S))) - *dt |= TEXTHTML; - else - *dt &= ~TEXTHTML; - - if (type && - 0 == strncasecmp (type, TEXTCSS_S, strlen (TEXTCSS_S))) - *dt |= TEXTCSS; - else - *dt &= ~TEXTCSS; + set_content_type (dt, type); if (opt.adjust_extension) { -- 2.3.4
From b6b6500a852d437ec52c8e49a600511723f32ab4 Mon Sep 17 00:00:00 2001 From: Hubert Tarasiuk <[email protected]> Date: Fri, 27 Mar 2015 15:35:57 +0100 Subject: [PATCH 2/2] Transform read_header label and goto into a loop * src/http.c (gethttp): Replace label and goto statement with a do loop. --- src/http.c | 101 ++++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 56 insertions(+), 45 deletions(-) diff --git a/src/http.c b/src/http.c index a127733..69c5cf5 100644 --- a/src/http.c +++ b/src/http.c @@ -2597,55 +2597,66 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy, /* warc_write_request_record has also closed warc_tmp. */ } + /* Repeat while we receive a 10x response code. */ + { + bool _repeat; -read_header: - head = read_http_response_head (sock); - if (!head) - { - if (errno == 0) - { - logputs (LOG_NOTQUIET, _("No data received.\n")); - CLOSE_INVALIDATE (sock); - request_free (req); - return HEOF; - } - else - { - logprintf (LOG_NOTQUIET, _("Read error (%s) in headers.\n"), - fd_errstr (sock)); - CLOSE_INVALIDATE (sock); - request_free (req); - return HERR; - } - } - DEBUGP (("\n---response begin---\n%s---response end---\n", head)); + do + { + head = read_http_response_head (sock); + if (!head) + { + if (errno == 0) + { + logputs (LOG_NOTQUIET, _("No data received.\n")); + CLOSE_INVALIDATE (sock); + request_free (req); + return HEOF; + } + else + { + logprintf (LOG_NOTQUIET, _("Read error (%s) in headers.\n"), + fd_errstr (sock)); + CLOSE_INVALIDATE (sock); + request_free (req); + return HERR; + } + } + DEBUGP (("\n---response begin---\n%s---response end---\n", head)); - resp = resp_new (head); + resp = resp_new (head); - /* Check for status line. */ - message = NULL; - statcode = resp_status (resp, &message); - if (statcode < 0) - { - char *tms = datetime_str (time (NULL)); - logprintf (LOG_VERBOSE, "%d\n", statcode); - logprintf (LOG_NOTQUIET, _("%s ERROR %d: %s.\n"), tms, statcode, - quotearg_style (escape_quoting_style, - _("Malformed status line"))); - CLOSE_INVALIDATE (sock); - resp_free (resp); - request_free (req); - xfree (head); - return HERR; - } + /* Check for status line. */ + message = NULL; + statcode = resp_status (resp, &message); + if (statcode < 0) + { + char *tms = datetime_str (time (NULL)); + logprintf (LOG_VERBOSE, "%d\n", statcode); + logprintf (LOG_NOTQUIET, _("%s ERROR %d: %s.\n"), tms, statcode, + quotearg_style (escape_quoting_style, + _("Malformed status line"))); + CLOSE_INVALIDATE (sock); + resp_free (resp); + request_free (req); + xfree (head); + return HERR; + } - if (H_10X (statcode)) - { - DEBUGP (("Ignoring response\n")); - resp_free (resp); - xfree (head); - goto read_header; - } + if (H_10X (statcode)) + { + xfree (head); + resp_free (resp); + _repeat = true; + DEBUGP (("Ignoring response\n")); + } + else + { + _repeat = false; + } + } + while (_repeat); + } xfree(hs->message); hs->message = xstrdup (message); -- 2.3.4
signature.asc
Description: OpenPGP digital signature
