0

I am sorting a two dimensional array a[n][2], with respect to a[i][0],a[i+1][0] breaking ties with non-decreasing a[i][1],a[i+1][1]. qsort is working fine with integer array but not with long long array.

Integer array code

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <math.h>
#include <limits.h>

int cmpfunc(const void* a, const void* b)
{
    int x = ((int*)a)[0] - ((int*)b)[0];
    if (x != 0) {
        return x;
    }
    return ((int*)a)[1] - ((int*)b)[1];
}

int main(int argc, char const* argv[])
{
    int n, i, j;
    scanf("%d", &n);
    int a[n][2];
    for (i = 0; i < n; i = i + 1) {
        scanf("%d %d", &a[i][0], &a[i][1]);
    }
    qsort(a, n, sizeof(a[0]), cmpfunc);
    for (i = 0; i < n; i = i + 1) {
        printf("%d %d\n", a[i][0], a[i][1]);
    }
    return 0;
}

long long array code

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <math.h>
#include <limits.h>

int cmpfunc(const void* a, const void* b)
{
    int x = ((int*)a)[0] - ((int*)b)[0];
    if (x != 0) {
        return x;
    }
    return ((int*)a)[1] - ((int*)b)[1];
}

int main(int argc, char const* argv[])
{
    int n, i, j;
    scanf("%d", &n);
    long long a[n][2];
    for (i = 0; i < n; i = i + 1) {
        scanf("%I64d %I64d", &a[i][0], &a[i][1]);
    }
    qsort(a, n, sizeof(a[0]), cmpfunc);
    for (i = 0; i < n; i = i + 1) {
        printf("%I64d %I64d\n", a[i][0], a[i][1]);
    }
    return 0;
}

Input:

5
4 3
4 2
4 1
4 1
4 1

Output for first code:

4 1
4 1
4 1
4 2
4 3

Output for second code:

4 2
4 1
4 1
4 1
4 3
Stargateur
  • 24,473
  • 8
  • 65
  • 91
Vishal
  • 33
  • 1
  • 5
  • 2
    In the case where you use `long long`, haven't you forgotten something? Especially in the comparison function? What is the actual type of `a` and `b`? – Some programmer dude Jul 06 '17 at 07:09
  • I suggest that you learn how to debug your code. Start by adding `printf()` statements in order to view the values of variables. – Code-Apprentice Jul 06 '17 at 07:09
  • @Someprogrammerdude How does it matter? the input is still in int range, it should work with int* cast also right? – Vishal Jul 06 '17 at 07:12
  • Your code don't make sense. You miss understand how `qsort()` works. – Stargateur Jul 06 '17 at 07:12
  • 1
    There is also the problem that `qsort` passes pointers to the elements to the comparison function, i.e. it passes `&a[0]` (using the `a` array in the `main` function) which have a type of `long long (*)[2]`. – Some programmer dude Jul 06 '17 at 07:13

3 Answers3

1

You still cast to int * in your compare function even though you changed the type of the data to long long.

Art
  • 19,807
  • 1
  • 34
  • 60
  • How does it matter? the input is still in int range. – Vishal Jul 06 '17 at 07:10
  • @Vishal And what if `sizeof(long long) != sizeof(int)`? Which is highly likely (`long long` is at least 64 bits, `int` is usually 32 bits even on 64-bit systems). – Some programmer dude Jul 06 '17 at 07:11
  • @Vishal Note that if you were casting a `long long` value to `int` and the value was within the `int` range, you would have no problems. However, you are casing an `long long *` to an `int *` which is very different. The range of the values is irrelevant when dealing with pointers. – Code-Apprentice Jul 06 '17 at 07:12
  • @Code-Apprentice I get it, It is working with long long * cast instead of int*, can you explain what would happen in the case of long long * to int* cast? – Vishal Jul 06 '17 at 07:19
  • @Vishal As others have mentioned, `long long` and `int` are different sizes. – Code-Apprentice Jul 06 '17 at 15:44
1

You actually have two issues: The first is the one with the invalid casting. The second is also about the invalid casting but for another reason.

As stated in one of my comments the qsort function passes pointers to the elements to the comparison function. If you haven an array arr then qsort will use something like &arr[0] for the arguments. That means if the data of the array is itself arrays then the arguments will be pointers to arrays. In your specific case the argument types are really long long (*)[2], not only long long *.

So the comparison function should look something like this instead:

int cmpfunc(const void* a_, const void* b_)
{
    long long (*a)[2] = (long long (*)[2]) a_;
    long long (*b)[2] = (long long (*)[2]) b_;

    long long result;

    if ((*a)[0] - (*b)[0] != 0)
        result = (*a)[0] - (*b)[0];
    else
        result = (*a)[1] - (*b)[1];

    if (result < 0)
        return -1;
    else if (result > 0)
        return 1;
    else
        return 0;
}
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • This is possible to keep const specifier ? yep, no need to cast just `long long const (*a)[2] = a_;` works. – Stargateur Jul 06 '17 at 07:21
  • The result of a difference between two `long long int` could not be in `int` range so this qsort implementation can't be used. Is this wrong? Also your code is not working properly for this question, you should try it because the real problem is in `scanf` placeholders. – simo-r Jul 06 '17 at 07:27
  • @BetaRunner Your correct that for the general case then the difference could be larger. Updated answer to store the result of the subtraction in a variable and use that to return an `int`. – Some programmer dude Jul 06 '17 at 07:33
  • Is `return (*a)[0] - (*b)[0];` valid in `int cmpfunc(...)` with `a` and `b` being of type `long long (*)[2]`? The difference is `long long` and I'm afraid casting to the return type `int` may truncate or wrap the result around zero. IMHO it's better to use nested `if` in this case to return `int` value `1` or `-1` depending on the difference being greater than or less than `(long long)0`, and default `return 0;` after all if-less and if-greater fail. **EDIT** Ooops, too long writing before save. – CiaPan Jul 06 '17 at 07:47
  • @Someprogrammerdude In the question code he's assuming that a `long long int` is `64-bit` which is not always true and he's also using `int` placeholders instead of `long long int`. I think that you should also show how to fix that(?) – simo-r Jul 06 '17 at 07:59
  • @BetaRunner `long long` is *at least* 64 bits. And I've yet to see a system with larger (e.g. 128 bit) `long long` type. VC++ (which have the `"%I64d"` format as a special extension) most certainly doesn't have non-64 bit `long long`. And that problem is for another question anyway. You might point it out for the OP as a comment to the question though. – Some programmer dude Jul 06 '17 at 08:02
  • @BetaRunner "he's assuming that a long long int is 64-bit" I think you are giving the OP too much credit here. It seems that he doesn't understand about the relationship between pointers and the size of the data being pointed to. – Code-Apprentice Jul 06 '17 at 15:49
0

If you enable warning while compiling (-Wall -pedantic for gcc) you should notice that you need different place holder for long long int and you can't assume that it's 64-bit. You also mis-cast arguments of compare function. Notice that the difference between two long long int can be outside the range of an int so you can't use that (the one I wrote) qsort() compare implementation. Here's the code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <math.h>
#include <limits.h>
int cmpfunc(const void *a,const void *b){
    long long int x=((long long int *)a)[0]-((long long int *)b)[0];
    if(x!=0){
        return x;
    }
    return ((long long int *)a)[1]-((long long int *)b)[1];
    }


int main(int argc, char const *argv[]){
    int n,i;
    scanf("%d",&n);
    long long a[n][2];
    for(i=0;i<n;i=i+1){
        scanf("%lld %lld",&a[i][0],&a[i][1]);
    }
    qsort(a,n,sizeof(a[0]),cmpfunc);
    for(i=0;i<n;i=i+1){
        printf("%lld %lld\n",a[i][0],a[i][1]);
    }
    return 0;
}
simo-r
  • 733
  • 1
  • 9
  • 13