0
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct student{
            int grade;
            int enrollCode;
}student;

typedef struct colVoidStar{
            int capacity;
            int num_itens_curr;
            void **arr;
            int current_pos;
}colVoidStar;

colVoidStar *colCreate(int capacity){
    if(capacity > 0){
        colVoidStar *c = malloc(sizeof(colVoidStar));
        if(c != NULL){
            c->arr = (void**)malloc(sizeof(void*)*capacity);
            if( c->arr != NULL){
                c->num_itens_curr = 0;
                c->capacity = capacity;
                return c;
            }
            free(c->arr);
        }
        free(c);
    }
    return NULL;
}


int colInsert(colVoidStar *c, void *item){
    if(c != NULL){
        if(c->num_itens_curr < c->capacity){
            c->arr[c->num_itens_curr] = (student*)item;
            c->num_itens_curr++;
            return 1;
        }
    }
    return 0;
}


void *colRemove(colVoidStar *c, void *key, int compar1(void* a, void* b)){
    int(*ptrCompar)(void*, void*) = compar1;
    student* eleRemoved;
    if(c != NULL){
        if(c->num_itens_curr > 0){
            int i = 0;
            for(i; i < c->num_itens_curr; i++){
                if(ptrCompar((void*)key, (void*)c->arr[i]) == 0){
                   eleRemoved = (student*)c->arr[i];
                   for(int j = i; j < c->num_itens_curr; j++){
                        c->arr[i] = c->arr[i + 1];
                        c->arr[i + 1] = 0;
                    }
                   return (void*)eleRemoved;
                }
                return NULL;
            }
        }
    }
    return NULL;
}

int compar1(void *a, void*b){
    int key;
    student *item;
    key = *(int*)a;
    item = (student*)b;
    return (int)(key - item->enrollCode);
}


int main(){
int finishProgram = 0, choose, capacity, returnInsert, removeEnroll;
colVoidStar *c;
student *a, *studentRemoved;
while(finishProgram != 9){
    printf("-----------------panel-----------------------\n");
    printf("Type: \n");
    printf("[1] to create a collection;\n");
    printf("[2] to insert a student;\n");
    printf("[3] to remove some student of collection;\n");
    printf("--------------------------------------------------------\n");
    scanf("%d", &choose);
    switch(choose){
        case 1:
            printf("Type the maximum of students the collection will have: \n");
            scanf("%d", &capacity);
            c = (colVoidStar*)colCreate(capacity);
            if(c == NULL){
                printf("Error in create collection!\n");
            }
            break;
        case 2:
            if(c->num_itens_curr < capacity){
                a = (student*)malloc(sizeof(student));
                printf("%d student:(type the Grade and the Enroll code, back-to-back)\n", c->num_itens_curr + 1);
                scanf("%d %d", &a->grade, &a->enrollCode);
                returnInsert = colInsert(c, (void*)a);
                if(returnInsert == 1){
                    for(int i = 0; i < c->num_itens_curr; i++){
                        printf("The student added has grade = %d e enrollCode = %d \n", (((student*)c->arr[i])->grade), ((student*)c->arr[i])->enrollCode);
                    }

                }else{
                    printf("the student wasn't added in the collection\n");
                }
            }else{
                printf("it's not possible to add more students to the colletion, since the limit of elements was reached!");
            }
            break;
        case 3:
            printf("Type an enrollcode to remove the student attached to it:\n");
            scanf("%d", &removeEnroll);
            studentRemoved = (student*)colRemove(c, &removeEnroll, compar1(&removeEnroll, c->arr[0]));
            if(studentRemoved != NULL)
                printf("the student removed has grade = %d and enrollcode %d.", studentRemoved->grade, studentRemoved->enrollCode);
            else
                printf("the number typed wasn't found");
            break;

    }
}
 return 0;
}

---> As you can realize, what I'm trying to do, at least at this point, is access and remove an item(student* that initially will assume a void* type) of a student's collection(void** arr) using a sort of enrollment code. However, I'm having problems with Segmentation Fault and can't understand why and how can solve them, hence my question up there. Debugging the code I found out the errors lies at: if(ptrCompar((void)key, (void**)*c->arr[i]) == 0) inside of Remove function and return (int)(key - item->matricula) inside of Compar1. Besides, if you can point me out some articles/documentations/whatever that helps me to understand how to cope with problems like that, I'll appreciate it a lot.

Bodo
  • 9,287
  • 1
  • 13
  • 29
  • It seems you should take this as an opportunity to learn how to use a *debugger*. Step through the code statement by statement, writing down all pointers and the addresses they point to, adding notes when they are passed to `free`. Then when the crash happens you see which pointer is being dereferenced (and how!) and compare to your list of pointers. Are you perhaps attempting to dereference a pointer passed to `free`? A null pointer? Or perhaps an uninitialized pointer? – Some programmer dude Jul 07 '21 at 12:42
  • 1
    The last argument of `colRemove(c, &removeEnroll, compar1(&removeEnroll, c->arr[0]));` does not match the formal parameter. The formal parameter is a function pointer, but the argument `compar1(&removeEnroll, c->arr[0])` is an `int`. Try `colRemove(c, &removeEnroll, compar1);`. – Ian Abbott Jul 07 '21 at 12:44
  • 1
    `colRemove` shouldn't be using `(student*)` at all if the `colVoidStar` functions are supposed to be generic. – Ian Abbott Jul 07 '21 at 12:51
  • The inner loop of `colRemove` is using the wrong variable to index `c->arr[]` and the loop termination condition is off by 1. – Ian Abbott Jul 07 '21 at 12:54
  • It would be easier to reproduce your problem if you would show what input you use, or even better if you would simplify your program by changing your `main` function to call the other functions with hard-coded data in a way to reproduce the problem. – Bodo Jul 07 '21 at 12:54
  • Understand that `void **` is very different from `void *`. In C, `void *` is a true, "generic" pointer type. It will automatically convert to/from any other pointer type. But `void **` is *not* a generic pointer-to-pointer. There is no generic pointer-to-pointer type in C. Strictly speaking, trying to use `void **` as a generic pointer-to-pointer type doesn't work. – Steve Summit Jul 07 '21 at 12:59
  • If you want to try to use `void **` as a "generic pointer to pointer", you will have to use explicit casts to tell the compiler, at any given point, what type you want to interpret the intermediate pointer as. (And the code you end up with will not be strictly portable.) – Steve Summit Jul 07 '21 at 13:02
  • @SteveSummit Detail: "It will automatically convert to/from any other pointer type." -->usually not a problem except `void *` to/from function pointers may lose information. Better as "... convert to/from any _object_ pointer type.". – chux - Reinstate Monica Jul 07 '21 at 13:08
  • Matteus Gutëmberg, Code does a lot of unneeded casting - that tends to hide errors. Most (or all?) `(void *)` casts not needed. – chux - Reinstate Monica Jul 07 '21 at 13:10
  • Matteus Gutëmberg `int key; ... key = *(int*)a;` is questionable as `int` may lose information. Consider `intptr_t key`. Note: `key - item->enrollCode` is UB on overflow. Unclear why code is subtracting when only a compare is needed. – chux - Reinstate Monica Jul 07 '21 at 13:14
  • Matteus Gutëmberg why does `colRemove()` never decrement `c->num_itens_curr`? – chux - Reinstate Monica Jul 07 '21 at 13:17

1 Answers1

0

Here are the problems I see in colRemove:

  1. (Not really a problem, just a matter of style) Although the function parameter int compar1(void* a, void* b) is OK, it is more conventional to use the syntax int (*compar1)(void* a, void* b).
  2. (Not really a problem) Having both compar1 and ptrCompar pointing to the same function is redundant. It is probably better to name the parameter ptrCompar to avoid reader's confusion with the compar1 function defined elsewhere in the code.
  3. The function is supposed to be general-purpose and shouldn't be using student* for the eleRemoved variable. Perhaps that was just for debugging? It should be void*.
  4. After the element to be removed has been found, the remaining code is all wrong:
    • c->num_itens_curr has not been decremented to reduce the number of items.
    • The code is accessing c->arr[i] and c->arr[i + 1] instead of c->arr[j] and c->arr[j + 1].
    • c->arr[j + 1] may be accessing beyond the last element because the loop termination condition is off by 1. This may be because c->num_itens_curr was not decremented.
    • The assignment c->arr[j + 1] = 0; is not really needed because all but the last element will be overwritten on the next iteration, and the value of the old last element does not matter because the number of items should be reduced by 1.
  5. (Not really a problem) There is unnecessary use of type cast operations in the function (e.g. casting void * to void *).

Here is a corrected and maybe improved version of the function (using fewer variables):

void *colRemove(colVoidStar *c, void *key, int (*ptrCompar)(void* a, void* b)){
    void* eleRemoved = NULL;
    if(c != NULL){
        int i;
        /* Look for item to be removed. */
        for(i = 0; i < c->num_itens_curr; i++){
            if(ptrCompar(key, c->arr[i]) == 0){
                /* Found it. */
                eleRemoved = c->arr[i];
                c->num_itens_curr--;    /* There is now one less item. */
                break;
            }
        }
        /* Close the gap. */
        for(; i < c->num_itens_curr; i++){
            c->arr[i] = c->arr[i + 1];
        }
    }
    return eleRemoved;
}

In addition, this call of colRemove from main is incorrect:

            studentRemoved = (student*)colRemove(c, &removeEnroll, compar1(&removeEnroll, c->arr[0]));

The final argument should be a pointer to the compar1 function, but the code is actually passing the result of a call to the compar1 function which is of type int. It should be changed to this:

            studentRemoved = (student*)colRemove(c, &removeEnroll, compar1);

or, removing the unnecessary type cast of the the void* to student*:

            studentRemoved = colRemove(c, &removeEnroll, compar1);

The colInsert function is also supposed to be general-purpose so should not use this inappropriate type cast to student*:

            c->arr[c->num_itens_curr] = (student*)item;

Perhaps that was also for debugging purposes, but it should just be using item as-is:

            c->arr[c->num_itens_curr] = item;

As pointed out by @chux in the comments on the question, the expression key - item->enrollCode in the return statement of compar1 may overflow. I recommend changing it to something like this:

   return key < item->enroleCode ? -1 : key > item->enrolCode ? 1 : 0;

or changing it to use this sneaky trick:

   return (key > item->enroleCode) - (key < item->enroleCode);
Ian Abbott
  • 15,083
  • 19
  • 33