On Fri, 2012-03-02 at 15:25 +0100, Jakub Hrozek wrote:
> On Fri, Mar 02, 2012 at 08:25:05AM -0500, Stephen Gallagher wrote:
> > On Fri, 2012-03-02 at 14:16 +0100, Jakub Hrozek wrote:
> > > Both krb5_child and ldap_child would emit a "child started"
message and
> > > only after that set up debugging to file. This might confuse users,
> > > because unless there is an error, the krb5_child.log might actually be
> > > empty.
> >
> > Nack. "[sssd[krb5_child[%d]]]", getpid() isn't dependent on
anything
> > that you don't know at this point. Just talloc_asprintf() it on NULL and
> > then steal it onto pd later.
> >
> > Also, please add NULL-checks for the talloc_asprintf() calls. If they
> > return NULL, just assign a static string "[sssd[ldap_child]]" or
> > "[sssd[krb5_child]]" without the PID.
> >
>
> OK, new patch is attached. We won't be able to free debug_prg_name if
> talloc_zero fails later, but that's not a big deal, the child process is
> not a long-running one.
>
Hmm, that's a good point. Coverity and valgrind will likely complain
about the leak as well. On further thought, it's probably alright to
just fail the krb5_child if that asprintf doesn't work, because if we're
in that serious of an OOM situation, chances are high that other, more
important things will be failing anyway.
So let's do that. Sorry for the repeated revisions.
> > >
> > > I'm thinking we might also add a couple of "tracing" DEBUG
messages so
> > > that we can follow the flow in the subprocess more easily.
> >
> > Please open an RFE.
>
>
https://fedorahosted.org/sssd/ticket/1225
Thanks.