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

Amos Jeffries squid3 at treenet.co.nz
Tue Dec 16 11:07:03 UTC 2014


-----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"


> * 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?

* 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.


Once those bits are sorted I will apply.

Amos
-----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-----


More information about the squid-dev mailing list