[SSSD] [PATCHES] Collection and ELAPI patches

Stephen Gallagher sgallagh at redhat.com
Thu Oct 29 15:32:55 UTC 2009


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/29/2009 11:24 AM, Dmitri Pal wrote:
> Stephen Gallagher wrote:
>> On 10/25/2009 11:25 PM, Dmitri Pal wrote:
>>> See comments in the patches.
>>> Patch 1: Improvements to trace macro
>>> Patch 2: New object - dynamic array with reference count (to build it do
>>> it inside its directory it is not hooked to the build tree in this
>> patch)
>>> Patch 3: Minor improvement to collection
>>
>>> Patch 4-6: A set of patches improving ELAPI. It is still work in
>>> progress and not the final state. I am just trying to slice things. View
>>> then as an early preview rather than something that needs to be pushed.
>>> Comments about those are welcome but again it is not even half baked
>>> work though it builds, runs, does what I want and does not have memory
>>> leaks.
>>
>>> Next steps:
>>> * Revise high level API to add error callbacks
>>> * Add sync and async variant of the IPA
>>> * Hook tevent for sync API
>>> * Internally enforce that the dispatcher is always async, i.e provide
>>> you own event loop in non advanced function
>>> * Revise sink <-> provider interface . Assume all sinks are async.
>>> * Wrap internal processing of the events on the sink level in such a way
>>> that the event travels between the sinks via event loop calls and
>>> callbacks
>>> * Update wiki with design changes
>>> * Develop man pages
>>
>>> These are significant changes that affect the code so I am reluctant to
>>> pass it over to anybody at this point.
>>> Plan to do as much as possible as soon as possible...
>>
>>
>>
>>
>>> _______________________________________________
>>> sssd-devel mailing list
>>> sssd-devel at lists.fedorahosted.org
>>> https://fedorahosted.org/mailman/listinfo/sssd-devel
>>
>> Patch 0001: I can't figure out why you're restricting the length of the
>> filename and line number in the trace macro. What's the rationale for
>> this? I think it's fine to leave this as '%s (%d)' rather than '%40s
>> (%4d)'. Otherwise, this patch looks fine.
> 
> First of all it was this way from the very beginning . I mean I used the
> special size modifiers.
> Main reason is that then the trace is nicely aligned in columns and
> easier to read.

Ah, I didn't realize that it enforced width. I thought it was just a
maximum.

> 
>>
>> Patch 0002: There is no reason for ref_array_next() and ref_array_prev()
>> to exist, as far as I can tell. Functionally, they're identical to
>> calling ref_array_get(ra, idx+/-1, &acptr). It would be different if you
>> were providing an iterator for the array.
> This is convenience and I like these two interfaces because they provide
> what I need.
> The iterator will be too much overhead for now.
> 

They're not convenient. They're a ridiculous duplication of code. If you
really want to have those functions, at least make them a macro wrapper
around ref_array_get() so that you don't have to maintain code in three
places.

#define red_array_next(ra, idx, acptr) ref_array_get(ra, idx+1, acptr)
#define red_array_prev(ra, idx, acptr) ref_array_get(ra, idx-1, acptr)

> 
>>
>> For this structure to be more useful, it should also have the ability to
>> delete elements from the array (maybe just the end, since this is
>> essentially a deque) and shrink the array.
>>
> Yes but I will not do it until it is required.
> Currently this functionality is left for future.
> 
> I have not created this interface to be 100% independent at least for now.
> Currently it is a helper and doe not need to act as a full blown API.
> As I started using it I see the needs for other features. If it will
> become an API on its own is yet to be seen.
> 
> It is good enough for the purpose and I do not have time to add more
> functionality to it.
> 
>> Patch 0003: Please update the comment in collection.h to denote that
>> passing NULL means to get a reference to the current collection.
>>
> 
> Will do.
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
> 


- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkrptaIACgkQeiVVYja6o6P2XACeLvNS41RsT1QEPnH1K3+Vlz9K
tKMAnjDtW6pRKNH5R5G6UvMMdbg9igq1
=WF3v
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list