rewrite statefile code for audio devices

In the words of the original author, it was 'crappy'.  I tend to
agree :)

The code has also been broken for at least the past few months,
and nobody bothered fixing it

The previous format it was overly complex: 5 lines to describe
each device.  The new format is one-line per-device:

audio_device_state:%d:%s

%d - 0 for disabled, any integer for enabled
%s - name of the device as specified in the config file,
whitespace and all

Incompatibilities:

* Output names are now _required_ to be unique.

This is required because the new format relies solely on the
name of the audio device.

Relying on the device IDs internal to MPD was a bad idea
anyways since the user usually has none or very little idea
how they're generated, and adding a new device or removing
one from a config would throw things off completely.

This is also just a Good Idea(TM) because it makes things
less confusing to users when they see it in their clients.

* Output states are not preserved from the previous format.

Not a big deal, since the previous code was never officially
released.  Also, it's been broken for months now, so I doubt
anybody would notice :)

git-svn-id: https://svn.musicpd.org/mpd/trunk@3928 09075e82-0dd4-0310-85a5-a0d7c8717e4f

src/audio.c

@@ -34,18 +34,10 @@
 #include <errno.h>
 #include <unistd.h>
 
-
+#define AUDIO_DEVICE_STATE	"audio_device_state:"
-/* crappy code by qball */
+#define AUDIO_DEVICE_STATE_LEN	19	/* strlen(AUDIO_DEVICE_STATE) */
-#define AUDIO_STATE_FILE_DEVICE_START		"audio_device_start"
-#define AUDIO_STATE_FILE_DEVICE_ID		"audio_device_id: "
-#define AUDIO_STATE_FILE_DEVICE_ENABLED		"audio_device_enabled: "
-#define AUDIO_STATE_FILE_DEVICE_NAME		"audio_device_name: "
-#define AUDIO_STATE_FILE_DEVICE_END		"audio_device_end"
-
 #define AUDIO_BUFFER_SIZE	2*MAXPATHLEN
-/* /crappy code */
+static void saveAudioDevicesState();
-
-
 
 static AudioFormat audio_format;
 
@@ -104,6 +96,9 @@ void initAudioDriver() {
 	param = getNextConfigParam(CONF_AUDIO_OUTPUT, param);
 
 	do {
+		AudioOutput *output;
+		int j;
+
 		if(audioOutputArraySize == AUDIO_MAX_DEVICES) {
 			ERROR("only up to 255 audio output devices are "
 					"supported");
@@ -115,13 +110,24 @@ void initAudioDriver() {
 		audioOutputArray = realloc(audioOutputArray,
 				audioOutputArraySize*sizeof(AudioOutput *));
 
-		audioOutputArray[i] = newAudioOutput(param);
+		output = newAudioOutput(param);
-
+		if(!output && param) {
-		if(!audioOutputArray[i] && param) {
 			ERROR("problems configuring output device defined at "
 					"line %i\n", param->line);
 			exit(EXIT_FAILURE);
 		}
+
+		/* require output names to be unique: */
+		for (j = i - 1; j >= 0; --j) {
+			if ( !strcmp( output->name,
+					audioOutputArray[j]->name) ) {
+				ERROR("output devices with identical "
+						"names: %s\n",
+						output->name);
+				exit(EXIT_FAILURE);
+			}
+		}
+		audioOutputArray[i] = output;
 	} while((param = getNextConfigParam(CONF_AUDIO_OUTPUT, param)));
 }
 
@@ -221,6 +227,7 @@ void finishAudioConfig() {
 void finishAudioDriver() {
 	int i;
 
+	saveAudioDevicesState();
 	for(i = 0; i < audioOutputArraySize; i++) {
 		finishAudioOutput(audioOutputArray[i]);
 	}
@@ -415,9 +422,6 @@ void printAudioDevices(FILE * fp) {
 	}
 }
 
-
-/* more qball crappy code */
-
 static char * getStateFile() {
 	ConfigParam * param = parseConfigFilePath(CONF_STATE_FILE, 0);
 
@@ -426,50 +430,78 @@ static char * getStateFile() {
 	return param->value;
 }
 
-
+static void saveAudioDevicesState() {
-
+	char *stateFile;
-void saveAudioDevicesState() {
+	FILE *fp;
 	int i;
-	char * stateFile = getStateFile();
 
-	if(stateFile) {
+	if (!(stateFile = getStateFile()))
-		FILE * fp;
+		return;
 
 	while(!(fp = fopen(stateFile,"a")) && errno==EINTR);
 	if(!fp) {
 		ERROR("problems opening state file \"%s\" for "
-				"writing: %s\n", stateFile,
+			"writing: %s\n", stateFile, strerror(errno));
-				strerror(errno));
 		return;
 	}
-		for(i = 0; i < audioOutputArraySize; i++) {
+
-			myfprintf(fp, "%s\n", AUDIO_STATE_FILE_DEVICE_START);
+	assert(audioOutputArraySize != 0);
-			myfprintf(fp, "%s%i\n", AUDIO_STATE_FILE_DEVICE_ID,	i);
+	for (i = audioOutputArraySize - 1; i >= 0; --i) {
-			myfprintf(fp, "%s%s\n", AUDIO_STATE_FILE_DEVICE_NAME,	audioOutputArray[i]->name);
+		myfprintf(fp, AUDIO_DEVICE_STATE "%d:%s\n",
-			myfprintf(fp, "%s%i\n", AUDIO_STATE_FILE_DEVICE_ENABLED,(int)pdAudioDevicesEnabled[i]);
+					(int)pdAudioDevicesEnabled[i],
-			myfprintf(fp, "%s\n", AUDIO_STATE_FILE_DEVICE_END);
+					audioOutputArray[i]->name);
 	}
 	while(fclose(fp) && errno==EINTR);
+}
+
+static void parse_audio_device_state(FILE *fp)
+{
+	char buffer[AUDIO_BUFFER_SIZE];
+	int i;
+
+	assert(audioOutputArraySize != 0);
+
+	while (myFgets(buffer,AUDIO_BUFFER_SIZE,fp)) {
+		char *c, *name;
+
+		if (strncmp(buffer,AUDIO_DEVICE_STATE,AUDIO_DEVICE_STATE_LEN))
+			continue;
+
+		c = strchr(buffer,':');
+		if (!c || !(++c))
+			goto errline;
+
+		name = strchr(c,':');
+		if (!name || !(++name))
+			goto errline;
+
+		for (i = audioOutputArraySize - 1; i >= 0; --i) {
+			if (!strcmp(name, audioOutputArray[i]->name)) {
+				pdAudioDevicesEnabled[i] = atoi(c);
+				break;
+			}
+		}
+		continue;
+errline:
+		/* nonfatal */
+		ERROR("invalid line in state_file: %s\n", buffer);
 	}
 }
 
 void readAudioDevicesState() {
-	char * stateFile = getStateFile();
+	char *stateFile;
 	FILE *fp;
 	struct stat st;
-	if(stateFile) {
-		char buffer[AUDIO_BUFFER_SIZE];
-
-
-
 
+	if (!(stateFile = getStateFile()))
+		return;
 	if(stat(stateFile,&st)<0) {
 		DEBUG("failed to stat state file\n");
 		return;
 	}
 	if(!S_ISREG(st.st_mode)) {
-			ERROR("state file \"%s\" is not a regular "
+		ERROR("state file \"%s\" is not a regular file\n",
-				"file\n",stateFile);
+							stateFile);
 		exit(EXIT_FAILURE);
 	}
 
@@ -480,50 +512,7 @@ void readAudioDevicesState() {
 			strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-
+	parse_audio_device_state(fp);
-		while(myFgets(buffer,AUDIO_BUFFER_SIZE,fp)) {
-			if(strncmp(buffer,AUDIO_STATE_FILE_DEVICE_START, strlen(AUDIO_STATE_FILE_DEVICE_START))==0) {
-				char *name = NULL;
-				int id = -1;
-				int enabled = 1;				
-				if(!myFgets(buffer,AUDIO_BUFFER_SIZE,fp)) {
-					ERROR("error parsing state file \"%s\"\n", stateFile);
-					exit(EXIT_FAILURE);
-				}
-				while(strcmp(buffer,AUDIO_STATE_FILE_DEVICE_END)) {
-					if(strncmp(buffer,AUDIO_STATE_FILE_DEVICE_ID, strlen(AUDIO_STATE_FILE_DEVICE_ID)) == 0 ) {
-						if(strlen(buffer) > strlen(AUDIO_STATE_FILE_DEVICE_ID))
-						{
-							id = atoi(&buffer[strlen(AUDIO_STATE_FILE_DEVICE_ID)]);
-						}
-					}
-					if(strncmp(buffer,AUDIO_STATE_FILE_DEVICE_ENABLED, strlen(AUDIO_STATE_FILE_DEVICE_ENABLED)) == 0 ) {
-						if(strlen(buffer) > strlen(AUDIO_STATE_FILE_DEVICE_ENABLED))
-						{
-							enabled = atoi(&buffer[strlen(AUDIO_STATE_FILE_DEVICE_ENABLED)]);
-						} 
-					}                     
-					if(!myFgets(buffer,AUDIO_BUFFER_SIZE,fp)) {
-						ERROR("error parsing state file \"%s\"\n", stateFile);
-						exit(EXIT_FAILURE);
-					}
-				}
-				if(id != -1)
-				{
-					/* search for same name here, can we trust id? */
-					if(id < audioOutputArraySize)
-					{
-						pdAudioDevicesEnabled[id] = enabled;
-					}	
-				}
-				if(name != NULL)
-				{
-					free(name);
-				}
-			}
-		}
-
 	fclose(fp);
-	}
 }
 

src/audio.h

@@ -75,8 +75,6 @@ int disableAudioDevice(FILE * fp, int device);
 
 void printAudioDevices(FILE * fp);
 
-/* qball's crappy code */
 void readAudioDevicesState();
-void saveAudioDevicesState();
 
 #endif

src/main.c

@@ -300,7 +300,6 @@ void startMainProcess() {
         if(pid>0) {
 		initInputStream(); 
 	        initReplayGainState();
-		/* qball crappy code */
 		readAudioDevicesState();
 
 		/* free stuff we don't need */
@@ -317,9 +316,6 @@ void startMainProcess() {
 		finishAudioConfig();
 		finishAudioDriver();
 	
-		/* qball crappy code */
-		saveAudioDevicesState();
-	
 		finishPaths();
 
 		kill(mainPid, SIGTERM);