[SSSD] [PATCH] krb5_child: set debugging sooner

Stephen Gallagher sgallagh at redhat.com
Tue Mar 6 16:46:53 UTC 2012


On Tue, 2012-03-06 at 17:36 +0100, Jakub Hrozek wrote:
> On Mon, Mar 05, 2012 at 08:42:42AM -0500, Stephen Gallagher wrote:
> > On Mon, 2012-03-05 at 13:08 +0100, Jakub Hrozek wrote:
> > > On Fri, Mar 02, 2012 at 09:39:18AM -0500, Stephen Gallagher wrote:
> > > > 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.
> > > 
> > > A new patch is attached.
> > 
> > Nack. With these new patches, you may jump to "done" before main_ctx has
> > been set. You need to initialize main_ctx to NULL.
> > 
> > Also, I'd prefer if we went to "done" instead of calling _exit(-1); if
> > the allocation of main_ctx fails.
> 
> Thank you, yet another patch attached.

Thanks for your patience; it has been rewarded.

Ack for master and sssd-1-8.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120306/28f89792/attachment.sig>


More information about the sssd-devel mailing list