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.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
