[libav-devel] [PATCH] http: Factorize the code by adding http_read_header().

Martin Storsjö martin at martin.st
Sun May 20 17:28:35 CEST 2012


On Sun, 20 May 2012, Samuel Pitoiset wrote:

> This function is used in order to read http header replies.
> ---
> libavformat/http.c |   78 +++++++++++++++++++++++++++-------------------------
> 1 file changed, 40 insertions(+), 38 deletions(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index c69e3f5..5b6877a 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -324,13 +324,35 @@ static inline int has_header(const char *str, const char *header)
>     return av_stristart(str, header + 2, NULL) || av_stristr(str, header);
> }
>
> +static int http_read_header(URLContext *h, int *new_location)
> +{
> +    HTTPContext *s = h->priv_data;
> +    char line[1024];
> +    int err = 0;
> +
> +    for(;;) {
> +        if (http_get_line(s, line, sizeof(line)) < 0)
> +            return AVERROR(EIO);
> +
> +        av_dlog(NULL, "header='%s'\n", line);
> +
> +        err = process_line(h, line, s->line_count, new_location);
> +        if (err < 0)
> +            return err;
> +        if (err == 0)
> +            break;
> +        s->line_count++;
> +    }
> +
> +    return err;
> +}
> +
> static int http_connect(URLContext *h, const char *path, const char *local_path,
>                         const char *hoststr, const char *auth,
>                         const char *proxyauth, int *new_location)
> {
>     HTTPContext *s = h->priv_data;
>     int post, err;
> -    char line[1024];
>     char headers[1024] = "";
>     char *authstr = NULL, *proxyauthstr = NULL;
>     int64_t off = s->off;
> @@ -403,19 +425,9 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>     s->chunksize = -1;
>
>     /* wait for header */
> -    for(;;) {
> -        if (http_get_line(s, line, sizeof(line)) < 0)
> -            return AVERROR(EIO);
> -
> -        av_dlog(NULL, "header='%s'\n", line);
> -
> -        err = process_line(h, line, s->line_count, new_location);
> -        if (err < 0)
> -            return err;
> -        if (err == 0)
> -            break;
> -        s->line_count++;
> -    }
> +    err = http_read_header(h, new_location);
> +    if (err < 0)
> +        return err;
>
>     return (off == s->off) ? 0 : -1;
> }
> @@ -603,10 +615,11 @@ static int http_proxy_open(URLContext *h, const char *uri, int flags)
>     HTTPContext *s = h->priv_data;
>     char hostname[1024], hoststr[1024];
>     char auth[1024], pathbuf[1024], *path;
> -    char line[1024], lower_url[100];
> +    char lower_url[100];
>     int port, ret = 0, attempts = 0;
>     HTTPAuthType cur_auth_type;
>     char *authstr;
> +    int new_loc;
>
>     h->is_streamed = 1;
>
> @@ -647,30 +660,19 @@ redo:
>     s->filesize = -1;
>     cur_auth_type = s->proxy_auth_state.auth_type;
>
> -    for (;;) {
> -        int new_loc;
> -        // Note: This uses buffering, potentially reading more than the
> -        // HTTP header. If tunneling a protocol where the server starts
> -        // the conversation, we might buffer part of that here, too.
> -        // Reading that requires using the proper ffurl_read() function
> -        // on this URLContext, not using the fd directly (as the tls
> -        // protocol does). This shouldn't be an issue for tls though,
> -        // since the client starts the conversation there, so there
> -        // is no extra data that we might buffer up here.
> -        if (http_get_line(s, line, sizeof(line)) < 0) {
> -            ret = AVERROR(EIO);
> -            goto fail;
> -        }
> -
> -        av_dlog(h, "header='%s'\n", line);
> +    /* Note: This uses buffering, potentially reading more than the
> +     * HTTP header. If tunneling a protocol where the server starts
> +     * the conversation, we might buffer part of that here, too.
> +     * Reading that requires using the proper ffurl_read() function
> +     * on this URLContext, not using the fd directly (as the tls
> +     * protocol does). This shouldn't be an issue for tls though,
> +     * since the client starts the conversation there, so there
> +     * is no extra data that we might buffer up here.
> +     */
> +    ret = http_read_header(h, &new_loc);
> +    if (ret < 0)
> +        goto fail;
>
> -        ret = process_line(h, line, s->line_count, &new_loc);
> -        if (ret < 0)
> -            goto fail;
> -        if (ret == 0)
> -            break;
> -        s->line_count++;
> -    }
>     attempts++;
>     if (s->http_code == 407 &&
>         (cur_auth_type == HTTP_AUTH_NONE || s->proxy_auth_state.stale) &&
> -- 
> 1.7.10.2

The change looks correct to me, and I tested both codepaths. Not sure if 
it's worth to apply right now or if we should wait until the changes 
you're working on that depend on this are good to go though - opinions?

// Martin


More information about the libav-devel mailing list