This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/1570/

On April 4th, 2014, 8:52 a.m. UTC, Michal Minar wrote:

src/software-dbus/sw-utils.c (Diff revision 3)
311
            if (array2) {
312
                for (k = 0; k < array2->len; k++) {
313
                    pk_det = g_ptr_array_index(array2, k);
314
                    if (strcmp(pk_package_get_id(pk_pkg),
315
                            pk_details_get_package_id(pk_det)) == 0) {
316
                        found = 1;
317
                        break;
318
                    }
319
                }
320
                if (!found) {
321
                    pk_det = NULL;
322
                }
323
            }
I don't see a reason for sorting the array while it the end it's iterated from the beginning until corresponding details are found.

What about having index (a2i = 0) initialized outside of the loop and incrementing by one for each array1 package:

if (array2) {
    do {
        pk_det = g_ptr_array_index(array2, a2i);
        cmpres = strcmp(pk_package_get_id(pk_pkg), pk_details_get_package_id(pk_det));
        if (cmpres > 0) {
            // this should not happen - we have spare unmatched package details
            ++a2i;
        } else if (cmpres < 0) {
            // no matching package details for current pk_pkg
            pk_det = NULL;
        }
    } while (cmpres > 0);
    ++a2i;
}
                
                
            

On April 4th, 2014, 9:51 a.m. UTC, Michal Minar wrote:

Once more and more correctly:

if (array2) {
    do {
        pk_det = g_ptr_array_index(array2, a2i);
        cmpres = strcmp(pk_package_get_id(pk_pkg), pk_details_get_package_id(pk_det));
        if (cmpres > 0) {
            // this should not happen - we have spare unmatched package details
            ++a2i;
        } else if (cmpres < 0) {
            // no matching package details for current pk_pkg
            pk_det = NULL;
        } else {
            // found a match
            ++a2i;
        }
    } while (cmpres > 0);
}

On April 4th, 2014, 10 a.m. UTC, Michal Minar wrote:

Still not robust enough, so for the third time :)

if (array2) {
    cmpres = 0;
    while (cmpres > 0 && a2i < array2->len) {
        pk_det = g_ptr_array_index(array2, a2i);
        cmpres = strcmp(pk_package_get_id(pk_pkg), pk_details_get_package_id(pk_det));
        if (cmpres > 0) {
            // this should not happen - we have spare unmatched package details
            ++a2i;
        } else if (cmpres < 0) {
            // no matching package details for current pk_pkg
            pk_det = NULL;
        } else {
            // found a match
            ++a2i;
        }
    }
}
Obviously not event the third attempt is correct. Initialize cmpres to something above zero.
/me ends with publishing untested code for today.

- Michal


On April 3rd, 2014, 4:33 p.m. UTC, Peter Schiffer wrote:

Review request for OpenLMI Developers.
By Peter Schiffer.

Updated April 3, 2014, 4:33 p.m.

Repository: openlmi-providers

Description

Software-dbus: implemented MemberOfSoftwareCollectionProvider

Diffs

  • src/software-dbus/LMI_MemberOfSoftwareCollectionProvider.c (dcae7910c2bfb0f628f802ffda010a7ae52873c2)
  • src/software-dbus/LMI_Software.h (6e434f95c43f27d9e378d891eccf318ee30c3ee2)
  • src/software-dbus/LMI_SoftwareIdentityProvider.c (09fbd93d2775145dcc5c57d160360d90371206b8)
  • src/software-dbus/sw-utils.h (0735259281281b8e49a5748a69aaccd1e09fec14)
  • src/software-dbus/sw-utils.c (3cfa1759773f141a4d02ba25bbf1846fa603879a)

View Diff