Weiheng Li Weiheng Li - 3 months ago 15
Linux Question

What's the wrong with my qsort in linux?

I am using Mac OS to implement part of function of command sort:
part of my code is below:

int compare(const void *p, const void *q) {
return strcmp(*(char **)p, *(char **)q);
}
void sort_file(char *filename, int unique, int reverse) {
/* TODO: Complete this function */
/* Note: you will probably need to implement some other functions */
char buf[1024][1024];
int i=0;
FILE * fp;
if(!(fp=fopen(filename,"r"))){
perror("Open error!");
exit(0);
}
while(fgets(buf[i],1024,fp)){
printf("%s",buf[i]);
i++;
}
qsort(buf, i, sizeof(char *), compare);
}


The result always show
segmentation fault: 11

Anyone can tell me what's the problem and how to modify it?

I still want to know, if I don't know the maximum size of code in one line and in the file, how to define my array?

I get the idea from this page:
http://www.anyexample.com/programming/c/qsort__sorting_array_of_strings__integers_and_structs.xml

Answer

You don't have an array of pointers; you have an array of arrays. Each element in your array of arrays has a specific, fixed stride of 1024 char. qsort needs to know that, and you're not telling it. First change this:

qsort(buf, i, sizeof(char *), compare);

to this:

qsort(buf, i, sizeof *buf, compare);

Now qsort knows how big each "thing" is in your array of char arrays.

Next, your comparator should be altered to account for the address being passed and what it is as it pertains to your array of arrays. Each address passed to the comparator is where an element lays. But your elements are each char[1024]. The address of some char[1024] isn't some char**, it is char(*)[1024]. There are no pointers to pointers involved here. Your comparator can simply be:

int compare(const void *p, const void *q) 
{
    return strcmp(p, q);
}

Next, there is no limiter in you control loop to prevent overflowing your array of arrays. In short, this:

while(fgets(buf[i],1024,fp))

should be this:

while(i < 1024 && fgets(buf[i],1024,fp))

and ideally, that 1024 should be expressed as a constant somewhere to avoid magic number sprinkling.

Finally, you're leaking an open FILE* in your function. Not a good plan. Make sure to fclose() your file pointer.