Re: Patch for squidclient

"Amos Jeffries" <squid3_at_treenet.co.nz> wrote in message
news:4C68DBC6.30403_at_treenet.co.nz...
> Markus Moeller wrote:
>>
>> "Alex Rousskov" <rousskov_at_measurement-factory.com> wrote in message
>> news:4C67F515.6080303_at_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.
> Use of a #define'd buffer size comes in handy here to replace sizeof(buf)
> and calculate with when needing
> ie snprintf(buf+len, BUFFER_SIZE-len, "%s"
>

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