On (04/04/14 15:18), Jakub Hrozek wrote: >On Thu, Apr 03, 2014 at 07:11:37PM +0200, Jakub Hrozek wrote: >> On Thu, Mar 20, 2014 at 05:53:31PM +0100, Lukas Slebodnik wrote: >> > On (20/03/14 17:21), Jakub Hrozek wrote: >> > >On Thu, Mar 20, 2014 at 05:00:00PM +0100, Sumit Bose wrote: >> > >> On Thu, Mar 20, 2014 at 04:20:59PM +0100, Lukas Slebodnik wrote: >> > >> > ehlo, >> > >> > >> > >> > debug_prg_name is used in debug_fn and it was allocated under >> > >> > talloc context "kr". The variable "kr" was removed before the last >> > >> > debug >> > >> > messages in function main. It is very little change that it will be >> > >> > overridden. >> > >> > >> > >> > It is possible to see this issue with exported environment variable >> > >> > TALLOC_FREE_FILL=255 >> > >> >> > >> I'm pretty sure the patch works as expected and fixes the isssue. But I >> > >> wonder if a different approach might be better? I think it does not make >> > >> sense to allocate debug_prg_name on a given talloc context but that it >> > >> would be better to just allocate it on NULL. This is e.g. done in the >> > >> ldap_child (here a talloc_free() is missing on exit but this would be a >> > >> different story). Then debug_prg_name can even be allocate before kr >> > >> and the debug messages for a failed allocation of kr can use the right >> > >> program name and not "sssd". >> > > >> > >I agree, because also given that krb5_child is a short lived process, >> > >we don't care too much about possible leaks. >> > No problem. >> > >> > New version attached. >> > >> > LS >> >> This version works for me, do you want to also add a talloc_free() on >> exit to be clean or do you also consider this not needed for short-lived >> processes? > >Thinking again, it would be nicer to explicitly free the string on child >exit. Yes, the leak it's not a big deal for a short-lived process but it >would read better.
With the first version of patch string was freed, because it was alocated under "kr" context. Now, you should decide which version do you want to push :-) LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
