Using qsort to sort unsigned integers

Question

The program that I am working on takes a file and parses it line by line then changes each line into a uint32_t and adds it into an array. From there I am supposed to sort the array with qsort(). I wrote my program how I thought it should be but when I test the code out it is saying that all the new uint32_ts are the same value. Is this because of a mistake when I change the string into the uint32_t? Would it be better to use strtoul? One last question, is the implementation of my qsort correct? (it compiles and says that it has sorted things but I'm not sure since my conversion to uint_32 is obviously not correct. Anyways here is the code:

int main(int argc, char* argv[]){
  char const* const fileName = argv[1];
  FILE* file = fopen(fileName, "r"); // should check the result
  char line[256];
  uint32_t parArray[256];
  int compar(const void *a, const void *b){
     const unsigned long long *x = a, *y = b;
     if(*x > *y)
       return 1;
     else
       return(*x < *y) ? -1: 0;
   }
  int lineCounter = 0; // starts at 0 for the array
  while(fgets(line, sizeof(line), file)){
     // parse all info here
     uint32_t t = (uint32_t) line;
     // build the array
     parArray[lineCounter]=t;
     lineCounter++;
     printf("Original: %s, Unsigned Int: %u\n", line,t); 
  }
  qsort(&parArray[0],lineCounter+1,sizeof(uint32_t*),compar);
  int i;
  for(i=0;i<lineCounter;i++){
    printf("%u\n",parArray[i]);
  }
  return 0;
}

Defining a function inside another function (here `compar` inside of `main`) is a gcc extension. If you want your code to be portable you shouldn't use this. Just move your definition of `compar` in front of `main`. — Jens Gustedt, Feb 09 '12 at 20:03

cnicutar · Answer 1 · 2012-02-09T21:23:11.117

3

uint32_t t = (uint32_t) line;

That's not the right way to convert a string to a number in C. You probably want to use strtoul instead. And your qsort is also wrong (wrong number of elements, wrong element size).

parArray[lineCounter] = strtoul(line, NULL, 10);

/* ... */
qsort(parArray, lineCounter, sizeof(uint32_t), compar);

You are defining your compar function inside another function. This is a GCC extension and you shouldn't use it unless you don't plan for portability.

edited Feb 09 '12 at 21:23

answered Feb 09 '12 at 19:51

cnicutar

178,505
25
365
392

thanks that was exactly right. Your explanation was much better than anything else that I read – Dan Bradbury Feb 09 '12 at 20:09
"Don't ever do that!" you know, the troglodytes slaving away in the GCC mines worked long and hard so that you could dismiss their extensions out of hand ;-) – Steve Jessop Feb 09 '12 at 21:21
Or at any rate lambda-lifted. – Steve Jessop Feb 13 '12 at 23:46

score 1 · Accepted Answer · answered Feb 09 '12 at 19:56

You're not parsing the lines as you read them. Casting line to a uint32_t simply takes the address of that array in memory. That explains why every line of the output is the same. You probably want to call strtoul(line, NULL, 10) or similar instead.

Also, your second parameter to qsort is off by one. lineCounter has the correct value at the time the loop terminates: the number of lines in the file. By adding one, you're introducing undefined behavior by reading past the populated values in the array.

Using qsort to sort unsigned integers

2 Answers2