Skip to content
This repository
Browse code

[settings] Fix memory corruption caused by resolution sorting

This is possibly the most subtle bug I've seen.
What I've seen over last couple of weeks is random memory corruption
failures when browsing the settings/video output window.

I tracked it down to the sort function in CWinSystemBase::ScreenResolutions.
But why? Everything looked fine.

Eventually spotted it. The compare function doesn't obey strict weak ordering.
std::sort considers a reasonable response to that is to corrupt memory.

Here's a description from someone else who hit a similar bug:
http://schneide.wordpress.com/2010/11/01/bug-hunting-fun-with-stdsort/
  • Loading branch information...
commit 238c86786f993acabfaae8681254b907560f1122 1 parent 9d8dfd5
popcornmix authored September 03, 2013

Showing 1 changed file with 3 additions and 1 deletion. Show diff stats Hide diff stats

  1. 4  xbmc/windowing/WinSystem.cpp
4  xbmc/windowing/WinSystem.cpp
@@ -149,9 +149,11 @@ static void AddResolution(vector<RESOLUTION_WHR> &resolutions, unsigned int addi
149 149
 
150 150
 static bool resSortPredicate(RESOLUTION_WHR i, RESOLUTION_WHR j)
151 151
 {
  152
+  // note: this comparison must obey "strict weak ordering"
  153
+  // a "!=" on the flags comparison resulted in memory corruption
152 154
   return (    i.width < j.width
153 155
           || (i.width == j.width && i.height < j.height)
154  
-          || (i.width == j.width && i.height == j.height && i.flags != j.flags) );
  156
+          || (i.width == j.width && i.height == j.height && i.flags < j.flags) );
155 157
 }
156 158
 
157 159
 vector<RESOLUTION_WHR> CWinSystemBase::ScreenResolutions(int screen, float refreshrate)

0 notes on commit 238c867

Please sign in to comment.
Something went wrong with that request. Please try again.