Your message dated Sun, 27 Apr 2008 23:51:38 +0100
with message-id <[EMAIL PROTECTED]>
and subject line xmms has been removed from Debian, closing #213406
has caused the Debian Bug report #213406,
regarding xmms: non-random shuffling
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact [EMAIL PROTECTED]
immediately.)


-- 
213406: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=213406
Debian Bug Tracking System
Contact [EMAIL PROTECTED] with problems
--- Begin Message ---
Package: xmms
Version: 1.2.8-2
Severity: minor
Tags: patch

Reportedly, the xmms playlist shuffling function does not shuffle
tracks very randomly.  Looking at the code, it is easy to see why.
xmms uses random() with of course only a single seeding with time(),
but even worse, it uses 'random() % RANGE' to cut down to a particular
range.  Any C text will warn against this, since you are throwing away
the high bits and leaving only low bits, which on many implementations
aren't all that random.

The following patch fixes this by using /dev/urandom where available
and at least random() / (RAND_MAX/range) where not.  Note that since
playlist shuffling only occurs between tracks, I was not terribly
worried about CPU usage, and I generally read more /dev/urandom data
than is strictly needed.

It is suitable for upstream, configure.in test and all.  Let me know
if I should send this or a version of it upstream, or if you would
like to handle it.

Peter

(Thanks to freenode user 'helix' for reporting the bug and testing the
fix.)

diff -u xmms-1.2.8/configure.in xmms-1.2.8/configure.in
--- xmms-1.2.8/configure.in
+++ xmms-1.2.8/configure.in
@@ -70,6 +70,23 @@
 AM_PATH_GTK(1.2.2,,AC_MSG_ERROR([*** GTK+ >= 1.2.2 not installed - please 
install first ***]),gthread)
 AC_PATH_PROG(XMMS_PATH,xmms,no)
 
+AC_MSG_CHECKING(for /dev/urandom)
+if test -c /dev/urandom
+then
+       AC_DEFINE(HAVE_DEV_URANDOM,1,[/dev/urandom is present])
+else
+       AC_MSG_RESULT(no)
+       AC_MSG_CHECKING(for /dev/random)
+       if test -c /dev/random
+       then
+               AC_DEFINE(HAVE_DEV_RANDOM,1,[/dev/random is present])
+               AC_MSG_RESULT(yes)
+       else
+               AC_MSG_RESULT(no)
+       fi
+fi
+
+
 LIBS_save=$LIBS
 LIBS="$LIBS $GTK_LIBS"
 SM_LIBS=""
only in patch2:
unchanged:
--- xmms-1.2.8.orig/xmms/Makefile.am
+++ xmms-1.2.8/xmms/Makefile.am
@@ -56,7 +56,7 @@
 getopt.c getopt1.c getopt.h \
 urldecode.c urldecode.h \
 dnd.h \
-mkdtemp.c
+mkdtemp.c random.c random.h
 
 EXTRA_DIST= xmms_logo.xpm xmms_mini.xpm xmms.desktop xmms.wmconfig
 
only in patch2:
unchanged:
--- xmms-1.2.8.orig/xmms/playlist.c
+++ xmms-1.2.8/xmms/playlist.c
@@ -543,7 +543,7 @@
                /* If there are entries */
                if (g_list_length(skinlist)) {
                        /* Get a random value to select the skin to use */
-                       int randval = random() % (g_list_length(skinlist) + 1);
+                       int randval = xmms_random(g_list_length(skinlist) + 1);
                        /* If the random value is 0, use the default skin */
                        /* Otherwise subtract 1 from the random value and */
                        /* select the skin */
@@ -1601,14 +1601,14 @@
        for (node = list, i = 0; i < len; node = g_list_next(node), i++)
                ptrs[i] = node;
 
-       j = random() % len;
+       j = xmms_random(len);
        list = ptrs[j];
        ptrs[j]->next = NULL;
        ptrs[j] = ptrs[0];
 
        for (i = 1; i < len; i++)
        {
-               j = random() % (len - i);
+               j = xmms_random(len - i);
                list->prev = ptrs[i + j];
                ptrs[i + j]->next = list;
                list = ptrs[i + j];
only in patch2:
unchanged:
--- xmms-1.2.8.orig/xmms/main.c
+++ xmms-1.2.8/xmms/main.c
@@ -3484,11 +3484,7 @@
        }
        parse_cmd_line(argc, argv, &options);
 
-#if defined(HAVE_SRANDOMDEV)
-       srandomdev();
-#else
-       srandom(time(NULL));
-#endif
+       xmms_random_init();
 
        read_config();
 
only in patch2:
unchanged:
--- xmms-1.2.8.orig/xmms/xmms.h
+++ xmms-1.2.8/xmms/xmms.h
@@ -81,6 +81,7 @@
 #include "sm.h"
 #include "dnd.h"
 #include "urldecode.h"
+#include "random.h"
 
 #include "config.h"
 
only in patch2:
unchanged:
--- xmms-1.2.8.orig/xmms/random.h
+++ xmms-1.2.8/xmms/random.h
@@ -0,0 +1,25 @@
+/*  XMMS - Cross-platform multimedia player
+ *  Copyright (C) 2003 by Peter Samuelson
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+#ifndef RANDOM_H
+#define RANDOM_H
+
+void xmms_random_init(void);
+unsigned long xmms_random(unsigned long range);
+void xmms_random_fini(void);
+
+#endif
only in patch2:
unchanged:
--- xmms-1.2.8.orig/xmms/random.c
+++ xmms-1.2.8/xmms/random.c
@@ -0,0 +1,89 @@
+/*  XMMS - Cross-platform multimedia player
+ *  random numbers
+ *  Copyright (C) 2003 by Peter Samuelson
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+
+#include "xmms.h"
+#include <stdlib.h>
+#include <time.h>
+
+#undef DEVRANDOM
+#ifdef HAVE_DEV_URANDOM
+# define DEVRANDOM "/dev/urandom"
+#else
+# ifdef HAVE_DEV_RANDOM
+#  define DEVRANDOM "/dev/random"
+# endif
+#endif
+
+#ifdef DEVRANDOM
+
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+static int random_fd = -1;
+void xmms_random_init(void)
+{
+       xmms_random_fini();
+       random_fd = open(DEVRANDOM, O_RDONLY);
+       if (random_fd < 0) {
+               perror("warning: cannot open " DEVRANDOM);
+               srandom(time(NULL));
+       }
+}
+void xmms_random_fini(void)
+{
+       if (random_fd >= 0) {
+               close(random_fd);
+               random_fd = -1;
+       }
+}
+unsigned long xmms_random(unsigned long range)
+{
+       unsigned long int buf;
+       int bytes;
+
+       if (random_fd < 0)
+               xmms_random_init();
+       if (random_fd >= 0 &&
+           read(random_fd, (char *)&buf, sizeof(buf)))
+               return buf % range;
+
+       return (random() / (RAND_MAX / range)) % range;
+}
+
+#else /* !DEVRANDOM */
+
+void xmms_random_init(void)
+{
+#if defined(HAVE_SRANDOMDEV)
+       srandomdev();
+#else
+       srandom(time(NULL));
+#endif
+}
+void xmms_random_fini(void)
+{
+       /* nothing to do */
+}
+unsigned long xmms_random(unsigned long range)
+{
+       return (random() / (RAND_MAX / range)) % range;
+}
+
+#endif /* DEVRANDOM */


--- End Message ---
--- Begin Message ---
Version: 1:1.2.10+20070601-1+rm

The xmms package has been removed from Debian testing, unstable and
experimental, so I am now closing the bugs that were still opened
against it.

For more information about this package's removal, read
http://bugs.debian.org/461309 . That bug might give the reasons why
this package was removed, and suggestions of possible replacements.

Don't hesitate to reply to this mail if you have any question.

Thank you for your contribution to Debian.

--
Marco Rodrigues
http://Marco.Tondela.org


--- End Message ---

Reply via email to