Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SortableVector: avoid qsort()ing C++ objects #17

Merged
merged 1 commit into from Jan 31, 2016

Conversation

Projects
None yet
2 participants
@lkundrak
Copy link
Contributor

lkundrak commented Jan 30, 2016

That's a sin to memcpy() a non-primitive type such as std::string. With
the current GCC libstdc++ implementation std::string inlines and copying
them directly causes no end of mayhem.

Valgrind just waved goodbye and exited through an open window.

SortableVector: avoid qsort()ing C++ objects
That's a sin to memcpy() a non-primitive type such as std::string. With
the current GCC libstdc++ implementation std::string inlines and copying
them directly causes no end of mayhem.

Valgrind just waved goodbye and exited through an open window.

Let's just get rid of the broken SortableVector class altogether, use
the standard std::sort interface, overload < instead of using a custom
comparator and implement swap to do the replaces correctly.

@lkundrak lkundrak force-pushed the lkundrak:lr-sort branch from 90c4730 to 3ef95f3 Jan 31, 2016

thp added a commit that referenced this pull request Jan 31, 2016

Merge pull request #17 from lkundrak/lr-sort
SortableVector: avoid qsort()ing C++ objects

@thp thp merged commit e9bd8b5 into thp:master Jan 31, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@thp

This comment has been minimized.

Copy link
Owner

thp commented Jan 31, 2016

Indeed!

}

bool
operator<(Collection a, Collection b)

This comment has been minimized.

Copy link
@thp

thp Jan 31, 2016

Owner

Come to think of it, couldn't this be bool operator<(const Collection &a, const Collection &b)?

This comment has been minimized.

Copy link
@lkundrak

lkundrak Feb 7, 2016

Author Contributor

Honestly, I have no idea. I don't really know C++ past the plain C point and have no idea what & does there. Sorry.

This comment has been minimized.

Copy link
@thp

thp Feb 8, 2016

Owner

& in this context denotes a reference (think pointer that's used syntactically like a value).

I've updated the two operator functions now to take a const reference, otherwise comparing two collections would probably copy the whole collection around for every call:

c30403c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.