CESNET / libnetconf

C NETCONF library
Other
113 stars 83 forks source link

error in nc_cpblts_add #189

Closed ntadas closed 8 years ago

ntadas commented 8 years ago
int i;
char *s, *p, = NULL;

if (capabilities == NULL || capability_string == NULL) {
    return (EXIT_FAILURE);
}

/* get working copy of capability_string where the parameters will be ignored */
s = strdup(capability_string);
if ((p = strchr(s, '?')) != NULL) {
    /* in following comparison, ignore capability's parameters */
    *p = 0;
}

/* find duplicities */
for (i = 0; i < capabilities->items; i++) {
    if (strncmp(capabilities->list[i], s, strlen(s)) == 0) {
        /* capability is already in the capabilities list, but
         * parameters can differ, so substitute the current instance
         * with the new one
         */
        free(capabilities->list[i]);
        if (p != NULL) {
            *p = '?';
        }
        capabilities->list[i] = s;
        return (EXIT_SUCCESS);
    }
}

In the code above the find for duplicates is not being done in correct way. As it can be seen in the GDB below if we have a model with a name as a subset of the model being added, this method will wrongly remove the capability.

(gdb) p capabilities->list[i] $164 = 0x7ffbac0098e0 "urn:xpto:os:networking?module=networking&revision=2015-11-19" (gdb) p s $165 = 0x7ffbac00a150 "urn:xpto:os:ne" (gdb)

the comparison should be done with the sub string of capabilities->list[i] also (divided by ? also) and the string s, but it should be ensured also that both strlen are equal.

Something like (code below note tested :) ):

int i;
char *s, *p = NULL, *s1, *p1 = NULL;

if (capabilities == NULL || capability_string == NULL) {
    return (EXIT_FAILURE);
}

/* get working copy of capability_string where the parameters will be ignored */
s = strdup(capability_string);
if ((p = strchr(s, '?')) != NULL) {
    /* in following comparison, ignore capability's parameters */
    *p = 0;
}

/* find duplicities */
for (i = 0; i < capabilities->items; i++) {

    /* get working copy of capability_string where the parameters will be ignored */
    s1 = strdup(capabilities->list[i]);
    if ((p1 = strchr(s1, '?')) != NULL) {
        /* in following comparison, ignore capability's parameters */
        *p1 = 0;
    }
    if (strlen(s1) == strlen(s) && (strncmp(s1, s, strlen(s)) == 0)) {
        /* capability is already in the capabilities list, but
         * parameters can differ, so substitute the current instance
         * with the new one
         */
        free(capabilities->list[i]);
        if (p != NULL) {
            *p = '?';
        }
        if (p1 != NULL) {
            *p1 = '?';
            free(s1);
        }
        capabilities->list[i] = s;
        return (EXIT_SUCCESS);
    }
    if (p1 != NULL) {
        *p1 = '?';
         free(s1);
    }
}
/* unhide capability's parameters */
if (p != NULL) {
    *p = '?';
}

Best Regards

rkrejci commented 8 years ago

Please check that it works for you, I have fixed it in a more simple way.

ntadas commented 8 years ago

seems fine, thanks