how to make a qsort function and sort array of pointers to structs

Question

i'm trying to make a qsort function from scratch that sorts an array of pointers to structs

this is the code i have right now

static void swap(int *a, int *b) {
  int tmp = *a;
  *a = *b;
  *b = tmp;
}

void _qsort(void* list, int list_len, int left, int right, 
            int(*comp)(const struct shpg_item *a, const struct shpg_item *b)) {
  void *vt, *v3; 
  int i, last, mid = (left + right) / 2; 
  if (left >= right) 
    return; 

  void* vl = (char*)(list + (left * list_len)); 
  void* vr = (char*)(list + (mid * list_len)); 
  swap(vl, vr); 
  last = left; 
  for (i = left + 1; i <= right; i++) { 

    // vl and vt will have the starting address  
    // of the elements which will be passed to  
    // comp function. 
    vt = (char*)(list + (i * list_len)); 
    if ((*comp)(vl, vt) > 0) { 
      ++last; 
      v3 = (char*)(list + (last * list_len)); 
      swap(vt, v3); 
    } 
  } 
  v3 = (char*)(list + (last * list_len)); 
  swap(vl, v3); 
  _qsort(list,list_len, left, last - 1, comp);
  trace_int(1);
  _qsort(list, list_len, last + 1, right, comp); 
}

void list_sort(struct shpg_item **list, int list_len,
               int(*comp)(const struct shpg_item *a, const struct shpg_item *b)) {
  _qsort(*list,list_len,0,(list_len-1),comp);
}

but this gives a segmentation fault error , can any one tell me why and help me ?

Answer 1

void * pointer addition

void * pointer addition is undefined behavior. But since the usual UB is OK, this may or may not be OP's trouble.

void _qsort(void* list, int list_len, int left, ...
    ...
    (list + (left * list_len))  // UB

Instead recommend casting before addition.

// void* vl = (char*)(list + (left * list_len)); 
void* vl = ((char*) list) + (left * list_len); 

Other issues may exist

Answer 2

I haven't check the entire code but your swap function seems wrong. Depending on the comment lines in your code;

// vl and vt will have the starting address  
// of the elements which will be passed to  
// comp function. 

if (list + (left * list_len)) and (list + (last * list_len)) are pointers to be swapped (pointers to a string or a struct, for example), your swap function decoration & your caller line should read as:

1. Swapping two integers, floats, doubles, etc (in general swapping values only):

void swap(int *a, int *b) {
    int t = *a;
    *a = *b;
    *b = t;
}
...
int x = 5;
int y = 3;
swap(&x, &y);

2. If you need to swap two pointers (a char * string or another type of pointer pointing to a struct), you can just swap pointer values without swapping the content pointed in the actual memory:

void swap(void **a, void **b) {
    void *t = *a;
    *a = *b;
    *b = t;
}
...
char *x = "some string";
char *y = "some other string";
swap(&x, &y);

Answer 3

I've included a working example in the middle part of this answer, and also added an example using qsort.

Taking a quick look at the code I see problem here:

void _qsort(void* list, ...

Since list is an array of pointers it should be:

void _qsort(void** list, ...

or

void _qsort(void* list[], ...

With this declaration, pointer arithmetic will not be an issue, for example, list+3 == &list[3] == pointer to the 3rd pointer in the array. There's no need to cast list, as void** list will work fine in the main part of the code. The only code that will do any casting is the caller's compare function.

You can choose to emulate qsort's compare function parameters using type void **: compare(list+i, list+j), but it would be simpler to use type void *: compare(list[i], list[j]).

Swap should use void** as parameters. The call would be

    swap(list+i, list+j)

/*   ... */

void swap(void **i, void **j){
void * t;
    t = *i;
    *i = *j;
    *j = t;
}

---

There are some comments about a void pointer possibly having a different size than a struct pointer or any type of data pointer, and that this could cause an issue. If this was true, then the C library function qsort() would not work because the first parameter for qsort is a void pointer, which will result in the caller's pointer being cast to a void pointer. In the caller's compare function, both parameters are const void pointers which the caller's compare function has to cast to the actual pointer types. With qsort() and the caller's compare function, parameters are being cast both to and from void pointers without issue.

C guarantees that a void pointer can be used to hold any type of data pointer, so in essence a void pointer is a generic data pointer (in 16 bit segment or selector environments, a generic "near" data pointer).

---

This is a working example, using typical Lomuto partition scheme (pivot = a[hi]):

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

typedef struct {
    int  data;
    char name[32];
}XMPL;

int cmpr(void * pi, void *pj)
{
    if(((XMPL *)pi)->data < ((XMPL *)pj)->data)
        return -1;
    if(((XMPL *)pi)->data > ((XMPL *)pj)->data)
        return  1;
    return 0;
}

void swap(void **i, void **j){
void * t;
    t = *i;
    *i = *j;
    *j = t;
}

void QuickSort(void **a, int lo, int hi, int(*cmpp)(void *, void *))
{
void *p;
int i, j;

    while(lo < hi){
        p = a[hi];
        i = lo;
        for(j = lo; j < hi; ++j){
            if((cmpp(a[j], p) < 0)){
                swap(a+i, a+j);
                ++i;
            }
        }
        swap(a+i, a+hi);
        if(i - lo <= hi - i){           /* avoid stack overflow */
            QuickSort(a, lo, i-1, cmpp);
            lo = i+1;
        } else {
            QuickSort(a, i+1, hi, cmpp);
            hi = i-1;
        }
    }
}

#define COUNT (1024)

int main(int argc, char**argv)
{
XMPL *ax;                               /* array of structures */
XMPL **pax;                             /* array of pointers to structures */
int i;

    ax =  malloc(COUNT * sizeof(XMPL));
    pax = malloc(COUNT * sizeof(void **));

    for(i = 0; i < COUNT; i++){         /* init structs, array of ptrs */
        ax[i].data = rand();
        pax[i] = ax+i;
    }

    QuickSort(pax, 0, COUNT-1, cmpr);

    for(i = 1; i < COUNT; i++){
        if(pax[i-1]->data > pax[i]->data){
            break;
        }
    }
    if(i == COUNT)
        printf("passed\n");
    else
        printf("failed\n");
    
    free(pax);
    free(ax);

    return(0);
}

---

Hoare parition scheme will probably be a bit faster. However, in this case, merge sort should be faster than quick sort. Merge sort does more moves but fewer compares than quick sort, and in this case, only pointers are being moved, while the compare involves an indirection via a pointer and a call to a compare function via a pointer.

---

Same basic code, but using qsort. Note that the cmpr() function needed one more dereference for each parameter.

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

typedef struct {
    int  data;
    char name[32];
}XMPL;

int cmpr(const void * pi, const void *pj)
{
    if((*(XMPL **)pi)->data < (*(XMPL **)pj)->data)
        return -1;
    if((*(XMPL **)pi)->data > (*(XMPL **)pj)->data)
        return  1;
    return 0;
}

#define COUNT (1024)

int main(int argc, char**argv)
{
XMPL *ax;                               /* array of structures */
XMPL **pax;                             /* array of pointers to structures */
int i;

    ax =  malloc(COUNT * sizeof(XMPL));
    pax = malloc(COUNT * sizeof(void **));

    for(i = 0; i < COUNT; i++){         /* init structs, array of ptrs */
        ax[i].data = rand();
        pax[i] = ax+i;
    }

    qsort(pax, COUNT, sizeof(XMPL *), cmpr);

    for(i = 1; i < COUNT; i++){
        if(pax[i-1]->data > pax[i]->data){
            break;
        }
    }
    if(i == COUNT)
        printf("passed\n");
    else
        printf("failed\n");
    
    free(pax);
    free(ax);

    return(0);
}