[squid-dev] [PATCH] add memory keytab option

Markus Moeller huaraz at moeller.plus.com
Tue Dec 16 23:36:01 UTC 2014


Hi Amos,

>"Amos Jeffries"  wrote in message news:54901257.6050100 at treenet.co.nz...
>
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>On 16/12/2014 1:16 p.m., Markus Moeller wrote:
>> Hi Amos,
>>
>> Thank you for the feedback and suggestions.    I did some cleanup
>> using cppcheck too.  Regarding the optarg check I was under the
>> impression that getopt just makes sure optarg is never NULL.  Isn't
>> that the case ?
>>
>> Thank you Markus
>>
>> "Amos Jeffries"  wrote in message
>> news:548E20C8.1030808 at treenet.co.nz...
>>
>> On 15/12/2014 8:31 a.m., Markus Moeller wrote:
>>> Hi Amos, Could you check and add the following patch please ?
>>> They should improve performance on high load systems by reducing
>>> disk access The patch does the following: [...]
>>
>>
>> in helpers/negotiate_auth/kerberos/negotiate_kerberos_auth.8: * all
>> instances if hypen (-) in man pages must be \-escaped. If any are
>> missed out the mand and groff tools corrupt the page contents. NP:
>> you can test syntax without having to install the script by running
>> "man ./negotiate_kerberos_auth.8" from the helper directory.
>
>You still have several of these unprotected hyphens in the SYNOPSIS
>and OPTIONS sections inside the parameter labels like "Keytab-Name"
>

OK. The man command worked fine, so I didn't notice the unprotected hyphens.

>
>> * krb5_free_kt_list() - lp and prev locals can be defined on first
>> use
>
>This is not done. Though taking another look it seems the for() loop
>should probably be replaced with a while()-loop :
>
>+  krb5_kt_list lp = list;
>+  while (lp) {
>+    krb5_error_code retval = krb5_kt_free_entry(context, lp->entry);
>+    safe_free(lp->entry);
>+    if (retval)
>+      return retval;
>+    krb5_kt_list prev = lp;
>+    lp = lp->next;
>+    xfree(prev);
>+  }
>+  return retval;
>
>I was wrong about the first of the free() though, there is a
>possibility the loop may stop releasing memory between the free(entry)
>and free(lp) so the first needs to be safe_free() to ensure the
>invalid entry pointer is cleared.
>  Is that actually desirable behaviour?
>  What happens to the rest of the lp list memory and entries?
>  Is it possible that lp->entry was NULL/invalid before the loop
>operations started?
>

I have to admit I took this section from the MIT ktutil tool

>* Should at least display some debug info/warning about when
>krb5_kt_free_entry() returns non-0 / error.
>
>
>NP: I see in the MIT documentation "DEPRECATED Use
>krb5_free_keytab_entry_contents instead.". That will probably lead to
>bug reports soon. though I am NOT asking for that to be fixed in this
>patch.
>

I had this check already in my other helper. So did the same here.

>
>Once those bits are sorted I will apply.
>
>Amos

Thank you for the quick response
Markus


>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v2.0.22 (MingW32)
>
>iQEcBAEBAgAGBQJUkBJXAAoJELJo5wb/XPRjQnMH/3pZKsGJyx7NLtQNYi9zyg5K
>UwrlKVlr11CNPrxhlc23LrUQeS5mqoxBPlkGNzkuq0vSqSweWNw6kVaqr2KdoIOs
>FBp0FoxKvx55w7K12xtzMeruf4bYOj5BofgQCKr/WunSYsiL2hQxRxRYu0xzbmoF
>tIb6A4ls9qOuW+Hv7W45koG6ZckosQdILLOCM4BkMbxL6mM0VWpz9sDAJ64NaOjA
>mHlJ128MV9kOMnx7d+Sy86D5dL7PVZhX5qscNzL7N6cQft5YG0lDIh5cKUTeJa67
>sR+WJZaMcHe+uIlhvb2iE3kQPbZNyxVwL1S3y8vZ0ABimYEe79K5OosyHByrrTw=
>=W6nr
>-----END PGP SIGNATURE-----
>_______________________________________________
>squid-dev mailing list
>squid-dev at lists.squid-cache.org
>http://lists.squid-cache.org/listinfo/squid-dev 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: trunk_kerberos_memory_keytab_3.patch
Type: application/octet-stream
Size: 29237 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20141216/f13bf378/attachment-0001.obj>


More information about the squid-dev mailing list