Re: [Gnash-commit] [SCM] Gnash branch, master, updated. release_0_8_9_final-1501-ga2efa2e

[Sandro Santilli]

On Fri, Jul 06, 2012 at 12:12:13AM +0000, Gabriele Giacone wrote:

> commit a2efa2eaa937110ac257c6b06a9b04904335a202
> Author: Gabriele Giacone <address@hidden>
> Date:   Fri Jul 6 02:01:13 2012 +0200
> 
>     Set http_proxy environment variable only if not already set.
>     
>     see bug #26713, authenticated proxy works by setting http_proxy with
>     credentials.
> 
> diff --git a/plugin/npapi/plugin.cpp b/plugin/npapi/plugin.cpp
> index dc597f1..af476e2 100644
> --- a/plugin/npapi/plugin.cpp
> +++ b/plugin/npapi/plugin.cpp
> @@ -1203,10 +1203,16 @@ nsPluginInstance::setupProxy(const std::string& url)
>          // no proxy
>      }
>      else if ( parts[0] == "PROXY" ) {
> -        if (setenv("http_proxy", parts[1].c_str(), 1) < 0) {
> -            gnash::log_error(
> -                "Couldn't set environment variable http_proxy to %s",
> -                nproxy);
> +        char *http_proxy=getenv("http_proxy");
> +        if (!http_proxy) {
> +            gnash::log_debug("Setting http_proxy to %s", parts[1].c_str());
> +            if (setenv("http_proxy", parts[1].c_str(), 1) < 0) {
> +                gnash::log_error(
> +                    "Couldn't set environment variable http_proxy to %s",
> +                    parts[1].c_str());
> +            }
> +        } else {
> +            gnash::log_debug("http_proxy already set to %s", http_proxy);
>          }
>      }
>      else {


Great to see you here :)
One suggestion: since there are no automated tests for these things it could
help having a comment in place for whoever would touch that code next.
Something explaining _why_ we don't want to override the env variable. 
Could be as easy as a link (full URL) to the bug item.

--strk;