Jakub Hrozek wrote:
> On 09/17/2009 06:08 AM, Dmitri Pal wrote:
> > Dmitri Pal wrote:
> >>> See patch comments for details.
> >>> All three are not massive.
> >>> 1) Comparison and sorting collections
>
> I don't understand why are you testing for
> "first->property[second->property_len] != '\0'" when testing for
> COL_CMPIN_PROP_DOT? Does that mean that comparing the exactly same
> properties with _BEG and COL_CMPIN_PROP_DOT would compare OK (In other
> words, I care about dot only with strings that are not equal)?
Ok think about the case when you have a flattened collection.
Imagine you have elements:
bar
barcode
bar.drink1
bar.drink2
With _BEG but not dot it will match all of them.
With _BEG and _DOT it will match "bar", "bar.drink1" and "bar.drink1"
And this is what I am trying to accomplish at least this is how
I envision it to be used. That being said there is no use case
so far and this functionality is not used. When we get into a real case
of _BEG, _MID and _END
the functionality may be adjusted.
>
> In comparing COL_TYPE_BINARY, after "if (first->length ==
> second->length)", I think you are missing "else result = NONZERO;"
>
Agree. Will fix.
> For other types, I'm wondering if the code could be somehow refactored
> (with a macro perhaps), it's essentially 6 blocks of the very same code
> except for the cast.
>
I will think about it. Macro is a good idea...
> The swapped != is certainly a bug:
> if ((res =! 0) && (out_flags == 0)) {
> if ((res =! 0) && (out_flags == 0)) {
>
Auch... Yes.
> In general, is it possible to use some other sorting algorithm than the
> one used? It appears to be a variant of bubble sort which is not very
> effective. Maybe qsort from the standard C library could be used, but
> I'm not sure how to deal with the flags..maybe a wrapper could be used
> (as qsort only works based on return value)
>
Sure. I needed something quick and dirty that I can put together quickly
to test the comparisons. I will open a ticket but so far it is not critical
since sorting is used only for testing comparisons.
> >>> 2) Taking part of the code from long module and putting it into a new
> >>> module for readability
>
> Ack, just moves code around
>
> >>> 3) Adding new functionality to the module created in previous patch
> >>>
>
> Looks good to me.
>
_______________________________________________
sssd-devel mailing list
sssd-devel@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/
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
