Remove some wacky leftover debugging.

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.

diff --git a/CHANGES b/CHANGES
index 14edbdd3103c4ee7c4fef51d98d7c5c91ed26e5f..782458e52e77ce8c02ac8b2fc7bcec52e6f34d31 100644 (file)
--- 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.
 
 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
 ** disorderfm
 
 There is a new command line tool called 'disorderfm' which is designed

diff --git a/server/rescan.c b/server/rescan.c
index a35ee98c47d37229d870440161ced4aeac53e7dc..3046a295fffe6df2b1ec790cea3e4e90f4e85b31 100644 (file)
--- 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]);
 
   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) {
 }
 
 int main(int argc, char **argv) {

diff --git a/server/trackdb.c b/server/trackdb.c
index 641913acc5aaf84e7f41bcfc122ef4d6688397d3..be1aea417a7bd6be6edc681ded10986e4d8567ab 100644 (file)
--- 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;
 
   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++)) {
   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:
    * 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((s = kvp_get(at, "_alias_for"))
        && !strcmp(s, track)) {
     case DB_NOTFOUND:

-      if(strstr(track, "Troggs"))
-        D(("accepting anyway"));

       *aliasp = d.vec;
     } else {
       *aliasp = d.vec;
     } else {

-      if(strstr(track, "Troggs")) {
-        D(("rejecting"));
-        D(("%s", track));
-        D(("%s", s ? s : "(null)"));
-      }

       *aliasp = 0;
     }
     return 0;
       *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,
 }
 
 /* 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,
 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;
 
   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;
   /* don't pick aliases - only pick the canonical form */
   if(gettrackdata(track, &t, &p, 0, 0, tid) == DB_LOCK_DEADLOCK)
     return DB_LOCK_DEADLOCK;