2

This is my Book structure and here is my code:

typedef struct book {
   char title[100];
   char author[20];
   int price;
} BOOK;

#define MAX_SIZE 10

int comparePrice(const void *bookA, const void *bookB);

int count = 0;

int main() {    
    BOOK *books[MAX_SIZE];    // here is  array of struct pointers
    while (1) {   
        char title[100] = "";
        char author[20] = "";
        int price = -1;
        //////////// printf select menu ////////////
        int selector = -1;
        scanf("%d", &selector);
        switch (selector) {
          case 1:
            while (getchar() != '\n');
            printf("--------input book data--------\n");
            printf("title :");
            gets(title);
            printf("author :");
            gets(author);

            printf("price :");
            scanf("%d", &price);

            books[count] = (BOOK *) malloc(sizeof(BOOK));
            memset(books[count], 0, sizeof(BOOK));
            strcpy(books[count]->title, title);
            strcpy(books[count]->author, author);
            books[count]->price = price;
            count++;
            break;
          case 4:
            printf("--------sorting price--------\n");
            qsort(books, count, sizeof(BOOK), comparePrice);
            for (int i = 0; i < count; ++i) {
                printf("%d. title: %s author: %s price: %d\n", i + 1, books[i]->title, books[i]->author, books[i]->price);
            }
            break;
          default:
            for (int i = 0; i < MAX_SIZE; ++i) {
                free(books[i]);
                books[i] = NULL;
            }
            return 0;
        }
    }
}

int comparePrice(const void *bookA, const void *bookB) {
    const BOOK *a = (const BOOK *)bookA;
    const BOOK *b = (const BOOK *)bookB;
    return b->price - a->price;
}

but qsort is not working select number 4 menu and this program stop. I tried debugging and found that a and b have unknown values. And EXC_BAD_ACCESS error occurs in the printf statement to print the sorting result. What I need to do for sorting.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
YIGeon
  • 57
  • 5
  • 1
    pssst, if you use `calloc`, you don't need to `memset` everything to 0 yourself. Any you're forgetting to `free`, so you have a memory leak. And [`gets` is evil](https://stackoverflow.com/q/1694036). `strcpy` is also pretty dangerous. – JHBonarius May 11 '21 at 17:29
  • 1
    nice edit, but there's no guarantee that the `books` array has `MAX_SIZE` elements. So you are possibly trying to free uninitialized pointers. Use `count`. – JHBonarius May 11 '21 at 17:36
  • @YIGeon Are you trying to sort in the descending order? – Vlad from Moscow May 11 '21 at 17:37
  • @VladfromMoscow Yes! – YIGeon May 11 '21 at 17:42

3 Answers3

1

The comparison function is incorrect: it gets pointers into the array, hence pointers to structure pointers. Furthermore, comparing integers by subtracting them does not work if the subtraction overflows which is possible when comparing very large values of opposite signs.

Here is a modified version:

int comparePrice(const void *aa, const void *bb) {
    const BOOK *a = *(const BOOK * const *)aa;
    const BOOK *b = *(const BOOK * const *)bb;
    /* sort in decreasing order of price */
    return (b->price > a->price) - (b->price < a->price);
}

The qsort() call is incorrect too. It should be:

qsort(books, count, sizeof(*books), comparePrice);

There are other problems in the code:

  • the array books is uninitialized so you should only free the pointers that have been allocated: change the last loop to

      while (count > 0) {
          free(books[--count]);
      }
    
  • do not use gets().

  • beware of the potential infinite loop: while (getchar() != '\n'); Always test for EOF:

      int c;
      while ((c = getchar()) != EOF && c != '\n')
          continue;
    
chqrlie
  • 131,814
  • 10
  • 121
  • 189
1

qsort passes pointers to the elements in the array to the compar function (which, in your case, are themselves pointers). So you want:

int comparePrice(const void *bookA, const void *bookB) {
    const BOOK *a = *(const BOOK **)bookA;
    const BOOK *b = *(const BOOK **)bookB;
    return b->price - a->price;
}
Chris Dodd
  • 119,907
  • 13
  • 134
  • 226
1

The array books should be initialized

BOOK *books[MAX_SIZE] = { NULL };

Otherwise this loop

for (int i = 0; i < MAX_SIZE; ++i) {
    free(books[i]);
    books[i] = NULL;
}

will invoke undefined behavior.

As you have an array of pointers then the call of qsort should look at least like

qsort( books, count, sizeof( BOOK * ), comparePrice );

On the other hand, he function comparePrice should be defined the following way

int comparePrice( const void *bookA, const void *bookB ) 
{
    const BOOK *a = *( const BOOK * const * ) bookA;
    const BOOK *b = *( const BOOK * const * ) bookB;
    
    return ( a->price < b->price ) - ( b->price < a->price );
}

Here is a simplified demonstrative program.

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

typedef struct book
{
    int price;
} BOOK;

int comparePrice( const void *bookA, const void *bookB ) 
{
    const BOOK *a = *( const BOOK * const * ) bookA;
    const BOOK *b = *( const BOOK * const * ) bookB;
    
    return ( a->price < b->price ) - ( b->price < a->price );
}

int main(void) 
{
    BOOK b1 = { 10 }, b2 = { 20 };
    BOOK *a[] = { &b1, &b2 };
    const size_t count = sizeof( a ) / sizeof( *a );
    
    for ( size_t i = 0; i < count; i++ )
    {
        printf( "%d ", a[i]->price );
    }
    
    putchar( '\n' );

    qsort( a, count, sizeof( BOOK * ), comparePrice );
    
    for ( size_t i = 0; i < count; i++ )
    {
        printf( "%d ", a[i]->price );
    }
    
    putchar( '\n' );
    
    return 0;
}

The program output is

10 20 
20 10
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335