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.
- 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.
- Taking part of the code from long module and putting it into a new
module for readability
Ack, just moves code around
- 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