Re: Patch for squidclient

[Markus Moeller]

"Amos Jeffries" <squ...@treenet.co.nz> wrote in message news:4c68dbc6.30...@treenet.co.nz...

Markus Moeller wrote:

"Alex Rousskov" <rouss...@measurement-factory.com> wrote in message news:4c67f515.6080...@measurement-factory.com...

On 08/14/2010 02:10 PM, Markus Moeller wrote:

Please find attached a patch to add Proxy- and WWW-Authenticate.

* GSSAPI_token not documented.

* check_gss_err not documented.

I did not see any function with documentation. I have added some lines now. What should be the format ?

Doxygen please:

/**
 * description...
 *
 * \retval 1  gssapi error
 * \retval 0  successful, no gssapi error.
 */

* It would be nice to remove gotos from the new code.

Done

* porxy misspelled; did not check for other typos

Fixed

* Please try to remove whitespace modifications that are unrelated to your patch.

I used formater.pl, which must have introduced them.

* Is tools/Makefile.in under revision control? If not, it should not be in the patch.

Not sure if it is under revision control, but I get it with rsync. I have removed it from the patch

The -h help text is mean to list the options in alphabetical order.

Also on the Usage: line. You can split at -m and -p like so:
   "[-k] [-l local-host] [-m method] "
+#if HAVE_GSSAPI
+  "[-n] [-N] "
+#endif

"[-p port] [-P file] [-t count] [-T timeout] [-u proxy-user] [-U www-user] "

Ok. Done

Please update the src/tools/squidclient.1 manual page with the new options.

Done

The "if (www_neg || proxy_neg)" around separate if for each case is redundant.

True. Sorry

In check_gss_err please use snprintf instead of sprintf.

  ie snprintf(buf+len, BUFFER_SIZE-len, "%s"

Use of a #define'd buffer size comes in handy here to replace sizeof(buf) and calculate with when needing

Thank you

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.6
  Beta testers wanted for 3.2.0.1

Markus

squid-3-head-tools-20100816.diff
Description: Binary data