Re: [PATCH] Fix leaks and check for newer libraries in Kerberos related helpers
On 08/26/2013 03:38 PM, Markus Moeller wrote: > Here is the update patch. > -if (pp && pp->next) { > -xfree(pp->next); > -pp->next = NULL; > -} > +safe_free(pp->next); This change will cause crashes on single-entry lists where pp is NULL. > -if (p == gdsp) { > -xfree(gdsp); > -gdsp = NULL; > -} > +safe_free(gdsp); > p = gdsp; This change will prevent cleanup of all entries except the very first one because the outer p-loop will terminate with p=gdsp making p NULL. Similar problems in another, similar part of the patch. You can completely remove an if-statement when using safe_free, but only where the if guard is the same as the one provided by safe_free: if (p) { xfree(p); p = NULL; } can be replaced with safe_free(p); However, if (something && p) { xfree(p); p = NULL; } can only be replaced with if (something) safe_free(p); Similarly, if (something) { xfree(p); p = NULL; } can only be replaced with if (something) safe_free(p); HTH, Alex.
Re: [PATCH] Fix leaks and check for newer libraries in Kerberos related helpers
Thank you for pointing this out. I was too radical :-(. Markus "Alex Rousskov" wrote in message news:521d0b73.3090...@measurement-factory.com... On 08/26/2013 03:38 PM, Markus Moeller wrote: Here is the update patch. -if (pp && pp->next) { -xfree(pp->next); -pp->next = NULL; -} +safe_free(pp->next); This change will cause crashes on single-entry lists where pp is NULL. -if (p == gdsp) { -xfree(gdsp); -gdsp = NULL; -} +safe_free(gdsp); p = gdsp; This change will prevent cleanup of all entries except the very first one because the outer p-loop will terminate with p=gdsp making p NULL. Similar problems in another, similar part of the patch. You can completely remove an if-statement when using safe_free, but only where the if guard is the same as the one provided by safe_free: if (p) { xfree(p); p = NULL; } can be replaced with safe_free(p); However, if (something && p) { xfree(p); p = NULL; } can only be replaced with if (something) safe_free(p); Similarly, if (something) { xfree(p); p = NULL; } can only be replaced with if (something) safe_free(p); HTH, Alex.