1

I have a struct declared like this

struct data
{
char * Date;
char * String;
};
struct data **RegArray = NULL;

int ArrayCount = 0;

I add new items to the array this way:

struct data **tmp = ( struct data ** )realloc( RegArray, ( ArrayCount + 1 ) * sizeof( struct data * ) );
if ( tmp == NULL )
{
    printf( "\nRealloc failed!" );
    return;
}
RegArray = tmp;

RegArray[ ArrayCount ] = ( struct data * )malloc( sizeof **RegArray );
if ( RegArray[ ArrayCount ] == NULL )
{
    printf( "\nMalloc failed!" );
    return;
}

RegArray[ ArrayCount ]->Date = _strdup( cDate );
RegArray[ ArrayCount ]->String = _strdup( cString );

ArrayCount++;

The function which compares the values:

int CompareByDate( const void *elem1, const void *elem2 )
{
//return ( ( data* )elem1 )->Date > ( ( data* )elem2 )->Date ? 1 : -1;
return strcmp( ( ( data* )elem1 )->Date, ( ( data* )elem2 )->Date );

}//CompareByDate

And finally I call qsort like this:

qsort( RegArray, ArrayCount-1, sizeof( data ), CompareByDate );

The problem is, that the data won't be sorted. So what am I doing wrong?

Thanks!

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
kampi
  • 2,362
  • 12
  • 52
  • 91
  • 4
    You may want to tag this as C rather than C++. There is little need to deal with this kind of thing in C++. – juanchopanza Dec 04 '13 at 14:38
  • [Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). – unwind Dec 04 '13 at 14:40
  • @unwind: I know, that I shouldn't do that, but otherwise Visual Studio won't compile. – kampi Dec 04 '13 at 14:41
  • 1
    Why use `struct data **` when you only allocate a single `data` structure? Why not simple `struct data * = realloc(...)`? Changing this would incidentally make your `qsort` *and* comparison function work as expected. – Some programmer dude Dec 04 '13 at 14:43
  • Is it okay to `realloc` before you `malloc`? Is that what you're doing? – Fiddling Bits Dec 04 '13 at 14:47
  • 1
    @BitFiddlingCodeMonkey If the pointer passed to `realloc` is `NULL` then it works just like `malloc`. – Some programmer dude Dec 04 '13 at 14:49
  • 2
    @kampi Then you're not compiling as C, but as C++. They're different languages, it's rarely a good idea to do this, even deliberately. – unwind Dec 04 '13 at 14:51

1 Answers1

1

In your qsort call and comparison function, you forget that you're dealing with an "array" of pointers. The easiest change is to not use an array of pointers:

struct data *RegArray = NULL;

/* ... */

struct data *tmp = realloc( RegArray, ( ArrayCount + 1 ) * sizeof( struct data ) );
if ( tmp == NULL )
{
    printf( "\nRealloc failed!" );
    return;
}
RegArray = tmp;

RegArray[ ArrayCount ].Date = _strdup( cDate );
RegArray[ ArrayCount ].String = _strdup( cString );

ArrayCount++;

This will make your qsort call (and comparison function) work as they are shown in the question.


If you don't want to change the code as outlined above, you have to change the qsort call and comparison function:

qsort( RegArray, ArrayCount-1, sizeof( data * ), CompareByDate );

/* ... */

int CompareByDate( const void *elem1, const void *elem2 )
{
    struct data **d1 = (struct data **) elem1;
    struct data **d2 = (struct data **) elem2;

    return strcmp((*d1)->Date, (*d2)->Date);
}
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Thank you! That solved the problem, however I made one more mistake. When calling qsort I used ArrayCount-1, and it should be only ArrayCount. – kampi Dec 04 '13 at 15:11