Hi Amos,

"Amos Jeffries"  wrote in message news:54901257.6050...@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.1030...@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@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Attachment: trunk_kerberos_memory_keytab_3.patch
Description: Binary data

_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to