Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

SortUtils: fix random sorting always being the same after restart #1448

Merged
merged 2 commits into from

4 participants

@Montellese
Owner

This fixes SortByRandom which relies on rand() but does not call srand() beforehand. Therefore everytime XBMC is restarted the numbers returned by srand() are the same (or rather the sequence of numbers is the same) which does not really provide proper random behaviour.

I'm not sure if this is the best approach which is why I post this PR. If it's good I'd like to merge it in ASAP as it is a fix.

@Montellese
Owner

OK I've added CUtil::GetRandomNumber() which uses rand_s() on win32 and rand_r() on anything else. I've only compile-tested (no runtime test done yet) this on ubuntu so far and will test it on win32 at home but not sure if rand_r() is also available on osx and freebsd? It should be in stdlib.h if it's present.

@davilla
Collaborator

RAND(3) BSD Library Functions Manual RAND(3)

NAME
rand, rand_r, srand, sranddev -- bad random number generator

LIBRARY
Standard C Library (libc, -lc)

SYNOPSIS
#include

 int
 rand(void);

 int
 rand_r(unsigned *seed);

 void
 srand(unsigned seed);

 void
 sranddev(void);
@Montellese
Owner

My initial approach for win32 didn't work out. I had to put #define _CRT_RAND_S which is necessary for rand_s() to be available from stdlib.h into the pch header file.

@MartijnKaijser

@Montellese
Confirmed working together with you JSON-RPC cleanup PR

@ghost

squash and pull

@Montellese Montellese was assigned
@Montellese Montellese merged commit 7c30250 into from
@fape

Hi! I try to compile to Android, but i got the following:
Util.cpp: In static member function 'static int CUtil::GetRandomNumber()':
Util.cpp:2622:30: error: 'rand_r' was not declared in this scope

Android (NDK) doesn't have rand_r.

@mikrohard mikrohard referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@mikrohard mikrohard referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@mikrohard mikrohard referenced this pull request from a commit in mikrohard/xbmc
@mikrohard mikrohard Android/Bionic doesn't implement rand_r
This fixes Android build after #1448 was merged.
92f6902
@LongChair LongChair referenced this pull request from a commit in plexinc/plex-home-theater-public
@LongChair LongChair Fix DimOnPause being wrongly disabled when we are in Dim screensaver …
…mode, fixes #1448
328b044
@LongChair LongChair referenced this pull request from a commit in plexinc/plex-home-theater-public
@LongChair LongChair Revert "Fix DimOnPause being wrongly disabled when we are in Dim scre…
…ensaver mode, fixes #1448"

This reverts commit 328b044.
84db705
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
20 xbmc/Util.cpp
@@ -32,11 +32,12 @@
#ifdef _LINUX
#include <sys/types.h>
#include <dirent.h>
-#include <stdlib.h>
#include <unistd.h>
#include <sys/wait.h>
#endif
+#include <stdlib.h>
+
#include "Application.h"
#include "Util.h"
#include "addons/Addon.h"
@@ -112,6 +113,10 @@ static uint16_t flashrampGreen[256];
static uint16_t flashrampBlue[256];
#endif
+#if !defined(TARGET_WINDOWS)
+unsigned int CUtil::s_randomSeed = time(NULL);
+#endif
+
CUtil::CUtil(void)
{
}
@@ -2607,3 +2612,16 @@ bool CUtil::CanBindPrivileged()
#endif //_LINUX
}
+int CUtil::GetRandomNumber()
+{
+#ifdef TARGET_WINDOWS
+ unsigned int number;
+ if (rand_s(&number) == 0)
+ return (int)number;
+#else
+ return rand_r(&s_randomSeed);
+#endif
+
+ return rand();
+}
+
View
10 xbmc/Util.h
@@ -189,6 +189,16 @@ class CUtil
static CStdString GetFrameworksPath(bool forPython = false);
static bool CanBindPrivileged();
+
+ /*!
+ * \brief Thread-safe random number generation
+ */
+ static int GetRandomNumber();
+
+#if !defined(TARGET_WINDOWS)
+private:
+ static unsigned int s_randomSeed;
+#endif
};
View
3  xbmc/utils/SortUtils.cpp
@@ -20,6 +20,7 @@
#include "SortUtils.h"
#include "URL.h"
+#include "Util.h"
#include "XBDateTime.h"
#include "settings/AdvancedSettings.h"
#include "utils/CharsetConverter.h"
@@ -395,7 +396,7 @@ string ByListeners(SortAttribute attributes, const SortItem &values)
string ByRandom(SortAttribute attributes, const SortItem &values)
{
CStdString label;
- label.Format("%i", rand());
+ label.Format("%i", CUtil::GetRandomNumber());
return label;
}
View
1  xbmc/win32/pch.h
@@ -1,4 +1,5 @@
#pragma once
+#define _CRT_RAND_S
#include <vector>
#include <map>
#include <string>
Something went wrong with that request. Please try again.