From: Andre Noll <maan@tuebingen.mpg.de>
Date: Mon, 14 Mar 2022 18:52:46 +0000 (+0100)
Subject: afs: Update dummy mood assumptions to reflect the reality.
X-Git-Tag: v0.7.1~34^2
X-Git-Url: http://git.tuebingen.mpg.de/?a=commitdiff_plain;h=19c6002ccd47b720b53410d0cbbecdae6ba80223;p=paraslash.git

afs: Update dummy mood assumptions to reflect the reality.

The code in afs.c assumes that loading the dummy mood always succeeds,
and this is even documented in change_current_mood(). However, this
has never been true because we call into osl library functions which
may fail for various reasons. In particular, if the server is started
without a database the attempt to load the dummy mood fails because
the audio file table does not exist.

The current code was not prepared to handle this case, and does stupid
things like storing the negative error code in *num_admissible and
returning success.

Fix this confusion by adjusting the documentation and letting
activate_mood_or_playlist() fail early. One of its callers,
init_admissible_files(), needs also be adjusted because it asserted
in its error path that the mood which failed to load was not the
dummy mood.

This is a benign bug because the most common way to hit this is
at startup on a fresh install when the database does not exist. In
this case the caller, init_admissible_files(), ignores the negative
num_admissible value.
---

diff --git a/afs.c b/afs.c
index b6cce36f..23ba2ad6 100644
--- a/afs.c
+++ b/afs.c
@@ -447,7 +447,6 @@ no_admissible_files:
 	return write_all(server_socket, buf, 8);
 }
 
-/* Never fails if arg == NULL */
 static int activate_mood_or_playlist(const char *arg, int *num_admissible,
 		char **errmsg)
 {
@@ -455,8 +454,13 @@ static int activate_mood_or_playlist(const char *arg, int *num_admissible,
 	int ret;
 
 	if (!arg) {
-		ret = change_current_mood(NULL, NULL); /* always successful */
 		mode = PLAY_MODE_MOOD;
+		ret = change_current_mood(NULL, errmsg);
+		if (ret < 0) {
+			if (num_admissible)
+				*num_admissible = 0;
+			return ret;
+		}
 	} else {
 		if (!strncmp(arg, "p/", 2)) {
 			ret = playlist_open(arg + 2);
@@ -615,10 +619,10 @@ static void init_admissible_files(const char *arg)
 {
 	int ret = activate_mood_or_playlist(arg, NULL, NULL);
 	if (ret < 0) {
-		assert(arg);
 		PARA_WARNING_LOG("could not activate %s: %s\n", arg,
 			para_strerror(-ret));
-		activate_mood_or_playlist(NULL, NULL, NULL);
+		if (arg)
+			activate_mood_or_playlist(NULL, NULL, NULL);
 	}
 }
 
diff --git a/mood.c b/mood.c
index 4e0a7e3d..5268e77f 100644
--- a/mood.c
+++ b/mood.c
@@ -857,8 +857,7 @@ void close_current_mood(void)
  *
  * If there is already an open mood, it will be closed first.
  *
- * \return Positive on success, negative on errors. Loading the dummy mood
- * always succeeds.
+ * \return Positive on success, negative on errors.
  *
  * \sa struct \ref afs_info::last_played, \ref mp_eval_row().
  */