[vlc-devel] [PATCH] support libproxy for http access

[Dominique Leuenberger]

Remi,

Thanks for the comment... as you might have realized I actually have no
clue about programming :-)

Nevertheless I tried to update the patch... I give some comments below.

On Sun, 2008-06-08 at 00:02 +0300, Rémi Denis-Courmont wrote:
> Le samedi 7 juin 2008 23:09:31 Dominique Leuenberger, vous avez écrit :
> > Addition of patch 0002: fixes up configure.ac to check if proxy.h exists
> > and uses it only in this case.
> 
> I assume libproxy does not support pkg-config, right?
> 

At least version 0.2.2 does not... I think it's planned for the
future... also a windows port is planned. The windows port should then
actually get it's info from the MSIE settings (thus closing one of the
bug's open in trac :-) )

> +       pxProxyFactory *pf = px_proxy_factory_new();
> +        if (!pf)
> +        {
> +               msg_Warn(p_access, "Loading of libproxy failed");
> 
> If libproxy actually failed to load, the linker would refuse to open the 
> plugin, so we would not reach this. The error message is probably misleading.
> 

True... I tried to change the error message.. and changed it to msg_Err,
it's not a warning.

> +                return 1;
> +        }
> +       char *buf ; sprintf(buf, "%s://%s", p_access->psz_access, 
> p_access->psz_path);
> 
> Does the compiler not warn about buf here?
> 

ups.. it did, buf being used uninitialized... fixed that

> +       msg_Warn(p_access, "asking libproxy about url '%s'", buf);
> 
> Don't use msg_Warn() here.
> 
yai... msg_Dbg us more useful here I think

> +       char **proxies = px_proxy_factory_get_proxies(pf, buf);
> +       if (proxies[0]) 
> +       {
> +           msg_Warn(p_access, "libproxy suggest to use '%s'", proxies[0]);
> 
> Don't use msg_Warn() here.
> 
Also used now msg_Dbg here.

> +           p_sys->b_proxy = true;
> +           vlc_UrlParse( &p_sys->proxy, proxies[0], 0);
> +       }
> +    }
> 
> ... Where are the resources freed?
> 
> 

Now? At the end...

Thank you very much for all your input. I believe it's the best way to
learn programming.

Dominique
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Link-libaccess_http_plugin-against-libproxy-to-autoc.patch
Type: application/mbox
Size: 1744 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20080608/5212c883/attachment.mbox>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Include-check-for-proxy.h-in-configure-and-only-link.patch
Type: application/mbox
Size: 1296 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20080608/5212c883/attachment-0001.mbox>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Free-memory-when-no-longer-used.patch
Type: application/mbox
Size: 1553 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20080608/5212c883/attachment-0002.mbox>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Replace-another-occurence-of-msg_Warn.patch
Type: application/mbox
Size: 932 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20080608/5212c883/attachment-0003.mbox>