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(a)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/