This is the mail archive of the
cygwin-apps
mailing list for the Cygwin project.
RE: [Patch] Proxy command-line argument
On 31 January 2008 08:05, Vincent Privat wrote:
Before we even get started: Thanks for your contribution! We're pretty
under resourced here and every little bit really helps :)
> Is the patch OK ?
It patched and compiles fine. I haven't had time to test it yet, but it
looks fairly obvious so I expect it'll work fine. Couple of questions:
+ std::string proxyString(((std::string)ProxyOption).c_str());
I'm not a C++ expert, so maybe I'm missing something, but if I read that
right you're taking ProxyOption (type StringOption), calling the std::string
conversion operator to return a std::string, getting a const char* from that
and using it to build a std::string. Why the trip via c_str? Can't you just
construct the new std::string from the return of the conversion operator
directly by invoking the std::string copy c-tor?
+ std::string proxyString((std::string)ProxyOption);
That seems simpler to me, but like I said I may be overlooking something.
+ if (proxyString.size())
+ {
+ NetIO::net_method = IDC_NET_PROXY;
If parsing the string fails now, we'll be in an invalid state later won't
we?
+ unsigned int pos = proxyString.find_last_of(':');
+ if (pos >= 0)
+ {
+ NetIO::net_proxy_host = strdup(proxyString.substr(0, pos).c_str());
+ if (pos < proxyString.size()-1)
+ {
+ std::string portString = proxyString.substr(pos+1,
proxyString.size()-(pos+1));
+ std::istringstream iss(portString, std::istringstream::in);
+ iss >> NetIO::net_proxy_port;
Do we really need to go to the lengths of building an istringstream here?
Wouldn't good old atoi be simpler? ;-)
+ }
+ }
+ }
The only one of these that I think actually matters is the assignment to
NetIO::net_method, which I think we should move inside the "if (pos >= 0)";
the rest I'm just curious about. I'll probably finish testing and commit it
at the weekend.
cheers,
DaveK
--
Can't think of a witty .sigline today....