[MPlayer-dev-eng] [PATCH 3/7] Use authorization field to pass server authentication through proxy

[Clément Bœsch]

On Sun, Nov 21, 2010 at 02:35:03PM +0100, Reimar Döffinger wrote:
> On Sun, Nov 21, 2010 at 02:24:26PM +0100, Clément Bœsch wrote:
> > diff --git a/stream/url.c b/stream/url.c
> > index 0469de9..90f41f7 100644
> > --- a/stream/url.c
> > +++ b/stream/url.c
> > @@ -58,6 +58,33 @@ URL_t *url_redirect(URL_t **url, const char *redir) {
> >    return res;
> >  }
> >  
> > +#ifdef __GNUC__
> > +static void make_noauth_url(URL_t *url, const char *fmt, ...) __attribute__ ((format(printf, 2, 3)));
> > +#endif
> > +
> > +static void make_noauth_url(URL_t *url, const char *fmt, ...)
> > +{
> > +    va_list va1, va2;
> > +    int len;
> > +
> > +    va_start(va1, fmt);
> > +    va_copy(va2, va1);
> > +    len = vsnprintf(NULL, 0, fmt, va1);
> > +    if (len < 0) {
> > +        url->noauth_url = NULL;
> > +        goto end;
> > +    }
> > +    len++;
> > +    url->noauth_url = malloc(len);
> > +    if (!url->noauth_url)
> > +        goto end;
> > +    vsnprintf(url->noauth_url, len, fmt, va2);
> > +
> > +end:
> > +    va_end(va1);
> > +    va_end(va2);
> > +}
> > +
> >  URL_t*
> >  url_new(const char* url) {
> >  	int pos1, pos2,v6addr = 0;
> > @@ -232,6 +259,17 @@ url_new(const char* url) {
> >  		strcpy(Curl->file, "/");
> >  	}
> >  
> > +	if (Curl->port)
> > +		make_noauth_url(Curl, "%s://%s:%d%s", Curl->protocol,
> > +				Curl->hostname, Curl->port, Curl->file);
> > +	else
> > +		make_noauth_url(Curl, "%s://%s%s", Curl->protocol,
> > +				Curl->hostname, Curl->file);
> > +	if (Curl->noauth_url == NULL) {
> > +		mp_msg(MSGT_NETWORK, MSGL_FATAL, MSGTR_MemAllocFailed);
> > +		goto err_out;
> > +	}
> 
> This seems like a really bad way to split it.
> 
> static int make_noauth_url(URL_t *url, char *dst, int dst_size)
> {
>     if (url->port)
>         return snprintf(dst, dst_size, "%s://%s:%d%s", url->protocol,
>                         url->hostname, url->port, url->file);
>     else
>         return snprintf(dst, dst_size, "%s://%s%s", url->protocol,
>                         url->hostname, url->file);
> }
> 
> 
> len = make_noauth_url(Curl, NULL, 0);
> if (len > 0) {
>     Curl->noauth_url = malloc(len);
>     if (!Curl->noauth_url) {
>     ...
>     }
>     make_noauth_url(Curl, Curl->noauth_url, len);
> }
> 
> Is far simpler.
> Or if you want to go with your approach it should be made generic
> in the form of a kind of "sprintf_alloc", without the URL_t argument
> but instead e.g. a char * return value - but I think that is overkill,
> and difficult to review (and I am still not a fan of the va_* stuff even
> though I guess that nowadays you can expect it to work correctly).

OK I see, then I'll commit this. Do you mind a reindent of
stream/{url,network,http}.c after this commit?

-- 
Clément B.
Not sent from a jesusPhone.