[SSSD] [PATCH] ELAPI Laying foundation for the async processing

Dmitri Pal dpal at redhat.com
Fri Sep 4 16:15:46 UTC 2009


Stephen Gallagher wrote:
> On 09/04/2009 11:18 AM, Dmitri Pal wrote:
> > Dmitri Pal wrote:
> >> Dmitri Pal wrote:
> >>   
> >>> See patch subject and commit comment.
> >>> This patch requires previous two patches I sent.
> >>>
> >>>   
> >>>     
> >> Self nack.
> >> I woke up today with a slightly better function names in mind.
> >> The updated patch will be out shortly.
> >>
> >>   
> > New patch is attached.
>
> >>>
> ------------------------------------------------------------------------
> >>>
> >>> _______________________________________________
> >>> sssd-devel mailing list
> >>> sssd-devel at lists.fedorahosted.org
> >>> https://fedorahosted.org/mailman/listinfo/sssd-devel
> >>>     
> >>
> >>   
>
>
> > ------------------------------------------------------------------------
>
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/sssd-devel
>
> Nack:
>
> elapi_async_ctx should be opaque (not specified in the elapi_async.h).
Yes I agree and I probably remove it in the next patch.
I do not have the file where it should reside yet.
I wanted the patch to be small. So that I can start developing other modules
where this code would be moved to.
> Your elapi_create_asctx() should allocate the memory for it and
> elapi_destroy_asctx() should deallocate memory for it.
Yes exactly. This is why they are create and destroy.
All my create-destroy functions do that.

> Similarly, elapi_check_asctx() should be an internal helper for
> elapi_create_asctx(), not a public function.

Most likely yes but I do not have the module to put it in yet.
I wanted to record it before I forget I wanted to have it.
I have a FIXME comment about it in the create dispatcher function.
>
> In struct elapi_dispatcher: You don't need async_mode. It is sufficient
> to check whether async_ctx is non-NULL. Also note that you removed the
> line that sets handle->async_mode = 1, so if you're relying on that,
> it's broken anyway.
Yes I am not decided yet. I will see how it goes. Checking for NULL
is exactly that I had in mind but something stopped me from removing it
completely. As you noticed I put a FIXME comment there.
At is an anchor I put for myself to come back later and sort it out.
I have a ticket to go back through the code and  look at all fixmes.

>
> What is the point of "offset" in struct elapi_dispatcher and
> elapi_event_ctx?
The offset of the time zone relative to UTC. I wanted to think more
about it.
This is something I need to handle and record in the "__loco__"
attribute of the event.
I have had a design discussion with Simo and John about it some time ago .
I am just trying to understand where it belongs so that there is not
much duplication on one
hand but you do not need to know more than you need to know on the
low levels of the interface and keep layers of interface as clean as
possible.

Bottom line:
I agree with your comments. I think I have already have the means to
track them address
them in the follow up patches.
My next step is to actually go back and add new interface to collection
to get
flattened names so instead of creating new modules and moving things around
I suggest taking this patch as is and leaving improvements to the follow
up work.

Keep in mind that this patch is third in the stack and the previous one
has not been looked at.
I am waiting for Jakub on that one.

Thanks!
_______________________________________________
sssd-devel mailing list
sssd-devel at lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/




More information about the sssd-devel mailing list