Crash while sorting a vector of pointers

[josh.townzen]

I'm writing a program that needs to display the contents of a std::map
of
Webuser objects, where each Webuser contains an ID number and the last
login
date in the form of a string. I'm using the ID number as the key for
each
map entry, but when displaying the map entries I need to sort them by
the
login date. I tried doing that by creating a vector of pointers to
Webuser
objects, filling that vector with the addresses to each Webuser in my
map, and
then sorting that vector based on the login date of the pointed-to
Webuser.
This is causing a crash during the sorting. I could find another way
to sort
the displayed entries, but I'd like to figure out what I'm doing
wrong.

This is the program below. I'm compiling it on a Win98 computer using
MinGW,
gcc version 3.4.5, with the command line
"gcc -o test.exe test.cpp -lstdc++ -g -Wall".
I've also compiled it on a Fedora computer using gcc version 4.1.1,
and the
resulting executable also crashed.

------------------------------------------------------------------------------

#include <stdio.h>

#include <iostream>
#include <fstream>

#include <string>
#include <vector>
#include <map>

class Webuser
{
public:
Webuser() :
id_(0) {}
Webuser(unsigned id, const std::string &last_login) :
id_(id),
last_login_(last_login) {}

void id(unsigned i) {id_ = i;}
void last_login(const std::string &l) {last_login_ = l;}

unsigned id() const {return id_;}
const std::string &last_login() const {return last_login_;}

private:
unsigned id_;
std::string last_login_;
};

class Webuser_sorter
{
public:
bool operator ()(const Webuser *user1, const Webuser *user2)
{
// We're sorting by the last login date, in reverse order
return user1->last_login() >= user2->last_login();
}
};

int main(int argc, char *argv[])
{
// Create a map of webusers and initialize it with some test data
std::map<unsigned, Webuser> webusers;
for (unsigned i = 0; i < 200; ++i)
webusers = Webuser(i, "2008-01-01");

// Create a vector of Webuser pointers, and fill it with the
addresses of
// each Webuser object in the webusers map.
std::vector<const Webuser *> sorted_webusers;
for (std::map<unsigned, Webuser>::const_iterator i =
webusers.begin();
i != webusers.end(); ++i)
{
sorted_webusers.push_back(&(i->second));
}

// Sort the vector of Webuser pointers
std::sort(sorted_webusers.begin(), sorted_webusers.end(),
Webuser_sorter());

// Display the sorted list of Webusers
for (unsigned i = 0; i < sorted_webusers.size(); ++i)
{
std::cout << sorted_webusers->id() << ": "
<< sorted_webusers->last_login() << std::endl;
}

return 0;
}

------------------------------------------------------------------------------

When I run the program, it crashes during the sort operation. When I
added
some debugging code to the beginning of the Webuser_sorter operator
():

bool operator ()(const Webuser *user1, const Webuser *user2)
{
// Two debugging lines added
std:fstream fout("debug.txt", std::ios::app);
fout << user1 << ", " << user2 << std::endl;

// We're sorting by the last login date, in reverse order
return user1->last_login() >= user2->last_login();
}

I found that one of the Webuser pointers (always user1 in my tests)
was
occasionally NULL after a couple hundred sorts. I don't know what I
could be
doing wrong that would cause std::sort to pass it a NULL pointer, but
I wanted
to see if anyone else saw a problem with my code before I started
looking into
whether it's a bug in gcc's standard c++ library.

 

[Pete Becker]

class Webuser_sorter
{
public:
bool operator ()(const Webuser *user1, const Webuser *user2)
{
// We're sorting by the last login date, in reverse order
return user1->last_login() >= user2->last_login();
}
};

This does not impose a strict weak ordering. For a Webuser_sorter
object w and two distinct Webuser objects a and b, both w(&a, &b) and
w(&b, &a) are true. Make the comparison > instead of >=. And while
you're at it, make operator() const.

 

[josh.townzen]

This does not impose a strict weak ordering. For a Webuser_sorter
object w and two distinct Webuser objects a and b, both w(&a, &b) and
w(&b, &a) are true. Make the comparison > instead of >=. And while
you're at it, make operator() const.

--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of "The
Standard C++ Library Extensions: a Tutorial and Reference
(www.petebecker.com/tr1book)

Thank you, using the greater-than operator fixed the problem.

 

[MathGis]

class Webuser_sorter

{
public:
    bool operator ()(const Webuser *user1, const Webuser *user2)
    {
        //  We're sorting by the last login date, in reverse order
        return user1->last_login() >= user2->last_login();
    }

};

I think your compare functor does not comply to the behaviour that is
expected by the sort function.
If user1->last_login() == user2->last_login(), your functor claims
that user1 should be ordered before user2 and also that user2 should
be before user1; how should that be sorted out?

Try:

return user1->last_login() > user2->last_login();