From 6aba3f6c378b62fbe5ef1bb1ad1ee1ef473e19c6 Mon Sep 17 00:00:00 2001
Message-Id: <6aba3f6c378b62fbe5ef1bb1ad1ee1ef473e19c6.1713885173.git.mdw@distorted.org.uk>
From: Mark Wooding <mdw@chiark.greenend.org.uk>
Date: Sun, 1 Jul 2007 20:37:14 +0100
Subject: [PATCH] Remove some wacky leftover debugging.
Organization: Straylight/Edgeware

From: Richard Kettlewell <rjk@greenend.org.uk>

check_suitable() now rejects tracks that don't belong to any
collection.  This situation can arise if you remove a collection from
the configuration.

A remaining bug is that even reproducible data for the tracks remains
in the database long after the collection has been removed.
---
 CHANGES          |  3 +++
 server/rescan.c  |  8 ++++++++
 server/trackdb.c | 20 ++++++++------------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/CHANGES b/CHANGES
index 14edbdd..782458e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -32,6 +32,9 @@ The 'pick' plugin has been abolished.  All the logic formerly done there
 is now built into the server, where it can be done much more
 efficiently.
 
+A bug where removing a collection (from the configuration) could cause a
+crash when random play was enabled has been fixed.
+
 ** disorderfm
 
 There is a new command line tool called 'disorderfm' which is designed
diff --git a/server/rescan.c b/server/rescan.c
index a35ee98..3046a29 100644
--- a/server/rescan.c
+++ b/server/rescan.c
@@ -270,6 +270,14 @@ static void do_all(void (*fn)(const struct collection *c)) {
 
   for(n = 0; n < config->collection.n; ++n)
     fn(&config->collection.s[n]);
+  /* TODO: we need to tidy up tracks from collections now removed.  We could do
+   * this two ways: either remember collections we think there are and spot
+   * their disappearance, or iterate over all tracks and gc any that don't fit
+   * into some collection.
+   *
+   * Having a way to rename collections would be rather convenient too but
+   * that's another kettle of monkeys.
+   */
 }
 
 int main(int argc, char **argv) {
diff --git a/server/trackdb.c b/server/trackdb.c
index 641913a..be1aea4 100644
--- a/server/trackdb.c
+++ b/server/trackdb.c
@@ -579,8 +579,6 @@ static int compute_alias(char **aliasp,
   int c, used_db = 0, slash_prefix, err;
   struct kvp *at;
 
-  if(strstr(track, "Troggs"))
-    D(("computing alias for %s", track));
   dynstr_init(&d);
   dynstr_append_string(&d, find_track_root(track));
   while((c = (unsigned char)*s++)) {
@@ -615,20 +613,11 @@ static int compute_alias(char **aliasp,
    * an alias) */
   switch(err = trackdb_getdata(trackdb_tracksdb, d.vec, &at, tid)) {
   case 0:
-    if(strstr(track, "Troggs"))
-      D(("found a hit for alias"));
     if((s = kvp_get(at, "_alias_for"))
        && !strcmp(s, track)) {
     case DB_NOTFOUND:
-      if(strstr(track, "Troggs"))
-        D(("accepting anyway"));
       *aliasp = d.vec;
     } else {
-      if(strstr(track, "Troggs")) {
-        D(("rejecting"));
-        D(("%s", track));
-        D(("%s", s ? s : "(null)"));
-      }
       *aliasp = 0;
     }
     return 0;
@@ -1162,7 +1151,8 @@ static int tag_intersection(char **a, char **b) {
 }
 
 /* Check whether a track is suitable for random play.  Returns 0 if it is,
- * DB_NOTFOUND if it or DB_LOCK_DEADLOCK. */
+ * DB_NOTFOUND if it is not or DB_LOCK_DEADLOCK if the database gave us
+ * that. */
 static int check_suitable(const char *track,
                           DB_TXN *tid,
                           char **required_tags,
@@ -1172,6 +1162,12 @@ static int check_suitable(const char *track,
   struct kvp *p, *t;
   const char *pick_at_random, *played_time;
 
+  /* don't pick tracks that aren't in any surviving collection (for instance
+   * you've edited the config but the rescan hasn't done its job yet) */
+  if(!find_track_root(track)) {
+    info("found track not in any collection: %s", track);
+    return DB_NOTFOUND;
+  }
   /* don't pick aliases - only pick the canonical form */
   if(gettrackdata(track, &t, &p, 0, 0, tid) == DB_LOCK_DEADLOCK)
     return DB_LOCK_DEADLOCK;
-- 
[mdw]