config file change! now 'port' is optional and 'db_file' is required!

also, should have better error reporting when failing to open playlist or
music directory's, or writing the db, etc

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

TODO

@@ -4,24 +4,16 @@
 	*) add 2-3 tree for sorted data structures
 	*) remove changes made to linked list for TagTracker
 
-*) debugging/errors
+*) add error codes for status->error
-	*) more info for failing to read/write db 
-	*) more info for stat'ing errors for music and playlist directory
-	*) add error codes for status->error
 
-*) config
+*) Cleanup Config File Code
-	*) make db_file required and port optional in the config file
-	*) remove command line config options, and require a config file
-	*) Cleanup Config File Code
-
-*) metadata todo
-	*) parsing of lame tags (including getting replaygain and gapless info)
-	*) implement apev2 and id3v1 tag reader from xmms-musepack plugin
-	*) only use libid3tag for id3v2 tags, use internal impl for id3v1 tags
 
 *) input plugins
 	*) Handle mp1 and mp2 files (including files with mp3 suffixes) 
 	*) add support for playing aac streams (gee, thanks icecast)
+	*) parsing of lame tags (including getting replaygain and gapless info)
+	*) implement apev2 and id3v1 tag reader from xmms-musepack plugin
+	*) only use libid3tag for id3v2 tags, use internal impl for id3v1 tags
 
 *) aduio output
 	*) allowing "pausing" of audio output devices

doc/mpdconf.example

@@ -3,9 +3,9 @@
 # Check the mpd man page, "man mpd".
 
 ##################### REQUIRED ###########################
-port 			"6600"
 music_directory 	"~/music"
-playlist_directory 	"~/.mpd/playlists"
+playlist_directory 	"~/music"
+db_file 		"~/.mpd/mpd.db"
 log_file 		"~/.mpd/mpd.log"
 error_file 		"~/.mpd/mpd.error"
 ##########################################################
@@ -112,10 +112,6 @@ audio_output {
 
 #################### OPTIONAL FILES ######################
 #
-# Location of DB file 
-#
-#db_file		"~/.mpd/mpd.db"
-#
 # The state file (if set) will be a file
 # for storing all current information 
 # (playlist, playing/paused, etc...) from
@@ -200,6 +196,8 @@ audio_output {
 
 ################ MISCELLANEOUS OPTIONS ###################
 #
+#port	"6600"
+#
 # This sets the metadata mpd will use, to disable all metadata, set to "none"
 # NOTE: comment's are disabled by default
 #

src/conf.c

@@ -361,7 +361,7 @@ BlockParam * getBlockParam(ConfigParam * param, char * name) {
 	return ret;
 }
 
-char * parseConfigFilePath(char * name, int force) {
+ConfigParam * parseConfigFilePath(char * name, int force) {
 	ConfigParam * param = getConfigParam(name);
 	char * path;
 
@@ -426,5 +426,5 @@ char * parseConfigFilePath(char * name, int force) {
 		param->value = newPath;
 	}
 
-	return param->value;
+	return param;
 }

src/conf.h

@@ -90,6 +90,6 @@ void registerConfigParam(char * name, int repeats, int block);
 
 BlockParam * getBlockParam(ConfigParam * param, char * name);
 
-char * parseConfigFilePath(char * name, int force);
+ConfigParam * parseConfigFilePath(char * name, int force);
 
 #endif

src/directory.c

@@ -46,6 +46,7 @@
 #include <stdio.h>
 #include <errno.h>
 #include <signal.h>
+#include <assert.h>
 
 #define DIRECTORY_DIR		"directory: "
 #define DIRECTORY_MTIME		"mtime: "
@@ -64,11 +65,9 @@
 #define DIRECTORY_RETURN_UPDATE         1
 #define DIRECTORY_RETURN_ERROR         -1
 
-Directory * mp3rootDirectory = NULL;
+static Directory * mp3rootDirectory = NULL;
 
-char * directory_db;
+static time_t directory_dbModTime = 0;
-
-time_t directory_dbModTime = 0;
 
 volatile int directory_updatePid = 0;
 
@@ -110,6 +109,15 @@ int inodeFoundInParent(Directory * parent, ino_t inode, dev_t device);
 
 int statDirectory(Directory * dir);
 
+static char * getDbFile() {
+	ConfigParam * param = parseConfigFilePath(CONF_DB_FILE, 1);
+
+	assert(param);
+	assert(param->value);
+
+	return param->value;
+}
+
 void clearUpdatePid() {
 	directory_updatePid = 0;
 }
@@ -207,8 +215,6 @@ int updateInit(FILE * fp, List * pathList) {
 		/* ignore signals since we don't want them to corrupt the db*/
 		ignoreSignals();
 		if(writeDirectoryDB()<0) {
-			ERROR("problems writing music db file, \"%s\"\n",
-					directory_db);
 			exit(DIRECTORY_UPDATE_EXIT_ERROR);
 		}
 		exit(DIRECTORY_UPDATE_EXIT_UPDATE);
@@ -753,7 +759,10 @@ int addSubDirectoryToDirectory(Directory * directory, char * shortname,
 int addToDirectory(Directory * directory, char * shortname, char * name) {
 	struct stat st;
 
-	if(myStat(name, &st)) return -1;
+	if(myStat(name, &st)) {
+		DEBUG("failed to stat %s: %s\n", name, strerror(errno));
+		return -1;
+	}
 
 	if(S_ISREG(st.st_mode) && hasMusicSuffix(name)) {
 		Song * song;
@@ -979,6 +988,7 @@ void sortDirectory(Directory * directory) {
 
 int writeDirectoryDB() {
 	FILE * fp;
+	char * dbFile = getDbFile();
 
 	DEBUG("removing empty directories from DB\n");
 	deleteEmptyDirectoriesInDirectory(mp3rootDirectory);
@@ -989,8 +999,12 @@ int writeDirectoryDB() {
 
 	DEBUG("writing DB\n");
 
-	while(!(fp=fopen(directory_db,"w")) && errno==EINTR);
+	while(!(fp=fopen(dbFile,"w")) && errno==EINTR);
-	if(!fp) return -1;
+	if(!fp) {
+		ERROR("unable to write to db file \"%s\": %s\n",
+				dbFile, strerror(errno));
+		return -1;
+	}
 
 	/* block signals when writing the db so we don't get a corrupted db*/
 	myfprintf(fp,"%s\n",DIRECTORY_INFO_BEGIN);
@@ -1006,12 +1020,17 @@ int writeDirectoryDB() {
 }
 
 int readDirectoryDB() {
-	FILE * fp;
+	FILE * fp = NULL;
+	char * dbFile = getDbFile();
         struct stat st;
 
 	if(!mp3rootDirectory) mp3rootDirectory = newDirectory(NULL, NULL);
-	while(!(fp=fopen(directory_db,"r")) && errno==EINTR);
+	while(!(fp=fopen(dbFile,"r")) && errno==EINTR);
-	if(!fp) return -1;
+	if(fp == NULL) {
+		ERROR("unable open db file \"%s\": %s\n", 
+				dbFile, strerror(errno));
+		return -1;
+	}
 
 	/* get initial info */
 	{
@@ -1090,7 +1109,7 @@ int readDirectoryDB() {
 	stats.numberOfSongs = countSongsIn(stderr,NULL);
 	stats.dbPlayTime = sumSongTimesIn(stderr,NULL);
 
-	if(stat(directory_db,&st)==0) directory_dbModTime = st.st_mtime;
+	if(stat(dbFile,&st)==0) directory_dbModTime = st.st_mtime;
 
 	return 0;
 }
@@ -1102,13 +1121,11 @@ void updateMp3Directory() {
                 return;
         case 1:
 	        if(writeDirectoryDB()<0) {
-		        ERROR("problems writing music db file, \"%s\"\n",
-                                        directory_db);
                         exit(EXIT_FAILURE);
 	        }
-                /* something was updated and db should be written */
                 break;
         default:
+                /* something was updated and db should be written */
 		ERROR("problems updating music db\n");
                 exit(EXIT_FAILURE);
 	}
@@ -1192,7 +1209,7 @@ void initMp3Directory() {
 	stats.numberOfSongs = countSongsIn(stderr,NULL);
 	stats.dbPlayTime = sumSongTimesIn(stderr,NULL);
 
-	if(stat(directory_db,&st)==0) directory_dbModTime = st.st_mtime;
+	if(stat(getDbFile(),&st)==0) directory_dbModTime = st.st_mtime;
 }
 
 Song * getSongDetails(char * file, char ** shortnameRet, 

src/directory.h

@@ -42,8 +42,6 @@ typedef struct _Directory {
 	DirectoryStat * stat;
 } Directory;
 
-extern char * directory_db;
-
 void readDirectoryDBIfUpdateIsFinished();
 
 void clearUpdatePid();

src/listen.c

@@ -39,7 +39,9 @@
 
 #define MAXHOSTNAME 	1024
 
-#define ALLOW_REUSE		1
+#define ALLOW_REUSE	1
+
+#define DEFAULT_PORT	6600
 
 int * listenSockets = NULL;
 int numberOfListenSockets = 0;
@@ -166,8 +168,25 @@ static int establishListen(unsigned int port, ConfigParam * param) {
 	return sock;
 }
 
-void establish(unsigned int port) {
+void listenOnPort() {
-	ConfigParam * param = getNextConfigParam(CONF_BIND_TO_ADDRESS, NULL);
+	int port = DEFAULT_PORT;
+	ConfigParam * param = getNextConfigParam(CONF_BIND_TO_ADDRESS,NULL);
+
+	{
+		ConfigParam * portParam = getConfigParam(CONF_PORT);
+
+		if(portParam) {
+			char * test;
+			port = strtol(portParam->value, &test, 10);
+			if(port <= 0 || *test != '\0') {
+				ERROR("%s \"%s\" specified at line %i is not a "
+						"positive integer", CONF_PORT,
+						portParam->value, 
+						portParam->line);
+				exit(EXIT_FAILURE);
+			}
+		}
+	}
 
 	do {
 		numberOfListenSockets++;

src/listen.h

@@ -26,7 +26,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-void establish(unsigned int port);
+void listenOnPort();
 
 void getConnections(fd_set * fds);
 

src/main.c

@@ -55,13 +55,6 @@
 #define USER_CONFIG_FILE_LOCATION	"/.mpdconf"
 
 typedef struct _Options {
-        char * portStr;
-        char * musicDirArg;
-        char * playlistDirArg;
-        char * logFile;
-        char * errorFile;
-        char * usr;
-        char * dbFile;
         int daemon;
         int stdOutput;
         int createDB;
@@ -103,12 +96,10 @@ void version() {
 void parseOptions(int argc, char ** argv, Options * options) {
         int argcLeft = argc;
 
-        options->usr = NULL;
         options->daemon = 1;
         options->stdOutput = 0;
         options->createDB = 0;
         options->updateDB = 0;
-        options->dbFile = NULL;
 
         if(argc>1) {
                 int i = 1;
@@ -158,55 +149,30 @@ void parseOptions(int argc, char ** argv, Options * options) {
                 }
         }
 
-        if(argcLeft==6) {
+        if(argcLeft<=2) {
-                options->portStr = argv[argc-5];
-                options->musicDirArg = argv[argc-4];
-                options->playlistDirArg = argv[argc-3];
-                options->logFile = argv[argc-2];
-                options->errorFile = argv[argc-1];
-                return;
-        }
-        else if(argcLeft<=2) {
-                int conf = 0;
                 if(argcLeft==2) {
 			readConf(argv[argc-1]);
-			conf = 1;
+			return;
 		}
                 else if(argcLeft==1) {
-                        FILE * fp;
+			struct stat st;
                         char * homedir = getenv("HOME");
                         char userfile[MAXPATHLEN+1] = "";
                         if(homedir && (strlen(homedir)+
-                                                strlen(USER_CONFIG_FILE_LOCATION)) <
+                        		strlen(USER_CONFIG_FILE_LOCATION)) <
-                                        MAXPATHLEN) {
+                                        	MAXPATHLEN) {
                                 strcpy(userfile,homedir);
                                 strcat(userfile,USER_CONFIG_FILE_LOCATION);
                         }
-                        if(strlen(userfile) && (fp=fopen(userfile,"r"))) {
+                        if(strlen(userfile) && (0 == stat(userfile,&st))) {
-                                fclose(fp);
                                 readConf(userfile);
-				conf = 1;
+				return;
                         }
-                        else if((fp=fopen(SYSTEM_CONFIG_FILE_LOCATION,"r"))) {
+                        else if(0 == stat(SYSTEM_CONFIG_FILE_LOCATION,&st)) {
-                                fclose(fp);
                                 readConf(SYSTEM_CONFIG_FILE_LOCATION);
-				conf = 1;
+				return;
                         }
                 }
-                if(conf) {
-                        options->portStr = forceAndGetConfigParamValue(
-					CONF_PORT);
-                        options->musicDirArg = 
-				parseConfigFilePath(CONF_MUSIC_DIR, 1);
-                        options->playlistDirArg = 
-				parseConfigFilePath(CONF_PLAYLIST_DIR, 1);
-                        options->logFile = parseConfigFilePath(CONF_LOG_FILE,1);
-                        options->errorFile = 
-				parseConfigFilePath(CONF_ERROR_FILE, 1);
-                        options->usr = getConfigParamValue(CONF_USER);
-                        options->dbFile = parseConfigFilePath(CONF_DB_FILE, 0);
-                        return;
-                }
         }
 
         usage(argv);
@@ -220,28 +186,20 @@ void closeAllFDs() {
         for(i = 3; i < fds; i++) close(i);
 }
 
-void establishListen(Options * options) {
+void changeToUser() {
-        int port;
+	ConfigParam * param = getConfigParam(CONF_USER);
-
+	
-        if((port = atoi(options->portStr))<0) {
+        if (param && strlen(param->value)) {
-                ERROR("problem with port number\n");
-                exit(EXIT_FAILURE);
-        }
-
-        if(options->createDB <= 0 && !options->updateDB) establish(port);
-}
-
-void changeToUser(Options * options) {
-        if (options->usr && strlen(options->usr)) {
                 /* get uid */
                 struct passwd * userpwd;
-                if ((userpwd = getpwnam(options->usr)) == NULL) {
+                if ((userpwd = getpwnam(param->value)) == NULL) {
-                        ERROR("no such user: %s\n", options->usr);
+                        ERROR("no such user \"%s\" at line %i\n", param->value,
+					param->line);
                         exit(EXIT_FAILURE);
                 }
 
                 if(setgid(userpwd->pw_gid) == -1) {
-                        ERROR("cannot setgid of user %s: %s\n", options->usr,
+                        ERROR("cannot setgid for user \"%s\" at line %i: %s\n", 					param->value, param->line,
                                         strerror(errno));
                         exit(EXIT_FAILURE);
                 }
@@ -250,9 +208,10 @@ void changeToUser(Options * options) {
                 /* init suplementary groups 
                  * (must be done before we change our uid)
                  */
-                if (initgroups(options->usr, userpwd->pw_gid) == -1) {
+                if (initgroups(param->value, userpwd->pw_gid) == -1) {
                         WARNING("cannot init suplementary groups "
-                                        "of user %s: %s\n", options->usr, 
+                                        "of user \"%s\" at line %i: %s\n", 
+					param->value, param->line, 
                                         strerror(errno));
                 }
 #endif
@@ -260,11 +219,13 @@ void changeToUser(Options * options) {
                 /* set uid */
                 if (setuid(userpwd->pw_uid) == -1) {
                         ERROR("cannot change to uid of user "
-                                        "%s: %s\n", options->usr, 
+                                        "\"%s\" at line %i: %s\n", 
+					param->value, param->line,
                                         strerror(errno));
                         exit(EXIT_FAILURE);
                 }
 
+		/* this is needed by libs such as arts */
 		if(userpwd->pw_dir) {
 			setenv("HOME", userpwd->pw_dir, 1);
 		}
@@ -272,6 +233,9 @@ void changeToUser(Options * options) {
 }
 
 void openLogFiles(Options * options, FILE ** out, FILE ** err) {
+	ConfigParam * logParam = parseConfigFilePath(CONF_LOG_FILE, 1);
+	ConfigParam * errorParam = parseConfigFilePath(CONF_ERROR_FILE, 1);
+	
         mode_t prev;
 
         if(options->stdOutput) {
@@ -282,15 +246,16 @@ void openLogFiles(Options * options, FILE ** out, FILE ** err) {
         /* be sure to create log files w/ rw permissions*/
         prev = umask(0066);
 
-        if(NULL==(*out=fopen(options->logFile,"a"))) {
+        if(NULL==(*out=fopen(logParam->value,"a"))) {
-                ERROR("problem opening file \"%s\" for writing\n",
+                ERROR("problem opening log file \"%s\" (config line %i) for "
-                                options->logFile);
+				"writing\n", logParam->value, logParam->line);
                 exit(EXIT_FAILURE);
         }
 
-        if(NULL==(*err=fopen(options->errorFile,"a"))) {
+        if(NULL==(*err=fopen(errorParam->value,"a"))) {
-                ERROR("problem opening file \"%s\" for writing\n",
+                ERROR("problem opening error file \"%s\" (config line %i) for "
-                                options->errorFile);
+				"writing\n", errorParam->value, 
+				errorParam->line);
                 exit(EXIT_FAILURE);
         }
 
@@ -298,23 +263,16 @@ void openLogFiles(Options * options, FILE ** out, FILE ** err) {
 }
 
 void openDB(Options * options, char * argv0) {
-        if(!options->dbFile) directory_db = strdup(rpp2app(".mpddb"));
-        else directory_db = strdup(options->dbFile);
-
         if(options->createDB>0 || readDirectoryDB()<0) {
                 if(options->createDB<0) {
                         ERROR("can't open db file and using \"--no-create-db\""
                                         " command line option\n");
-			ERROR("try running \"%s --create-db\"\n",
+			ERROR("try running \"%s --create-db\"\n", argv0);
-					argv0);
                         exit(EXIT_FAILURE);
                 }
                 flushWarningLog();
                 initMp3Directory();
-                if(writeDirectoryDB()<0) {
+                if(writeDirectoryDB()<0) exit(EXIT_FAILURE);
-                        ERROR("problem opening db for reading or writing\n");
-                        exit(EXIT_FAILURE);
-                }
 		if(options->createDB) exit(EXIT_SUCCESS);
         }
 	if(options->updateDB) {
@@ -372,8 +330,7 @@ void setupLogOutput(Options * options, FILE * out, FILE * err) {
                         exit(EXIT_FAILURE);
                 }
 
-                myfprintfStdLogMode(out, err, options->logFile,
+                myfprintfStdLogMode(out, err);
-                                options->errorFile);
                 flushWarningLog();
         }
 
@@ -408,13 +365,13 @@ int main(int argc, char * argv[]) {
 	initTagConfig();
         initLog();
 
-        establishListen(&options);
+        if(options.createDB <= 0 && !options.updateDB) listenOnPort();
 
-        changeToUser(&options);
+        changeToUser();
 
         openLogFiles(&options, &out, &err);
 
-	initPaths(options.playlistDirArg,options.musicDirArg);
+	initPaths();
 	initPermissions();
         initReplayGainState();
 

src/myfprintf.c

@@ -20,6 +20,7 @@
 #include "interface.h"
 #include "path.h"
 #include "log.h"
+#include "conf.h"
 
 #include <stdarg.h>
 #include <sys/param.h>
@@ -57,14 +58,12 @@ void blockingWrite(int fd, char * string, int len) {
 	}
 }
 
-void myfprintfStdLogMode(FILE * out, FILE * err, char * outFilename,
+void myfprintfStdLogMode(FILE * out, FILE * err) {
-                char * errFilename) 
-{
 	myfprintf_stdLogMode = 1;
 	myfprintf_out = out;
 	myfprintf_err = err;
-        myfprintf_outFilename = prependCwdToPathDup(outFilename);
+        myfprintf_outFilename = getConfigParamValue(CONF_LOG_FILE);
-        myfprintf_errFilename = prependCwdToPathDup(errFilename);
+        myfprintf_errFilename = getConfigParamValue(CONF_ERROR_FILE);
 }
 
 void myfprintf(FILE * fp, char * format, ... ) {

src/myfprintf.h

@@ -23,8 +23,7 @@
 
 #include <stdio.h>
 
-void myfprintfStdLogMode(FILE * out, FILE * err, char * outFilename,
+void myfprintfStdLogMode(FILE * out, FILE * err);
-                char * errFilename);
 
 void myfprintf(FILE * fp, char * format, ... );
 

src/path.c

@@ -28,6 +28,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <errno.h>
+#include <dirent.h>
 
 #ifdef HAVE_LOCALE
 #ifdef HAVE_LANGINFO_CODESET
@@ -110,36 +111,52 @@ char * getFsCharset() {
 	return fsCharset;
 }
 
-void initPaths(char * playlistDirArg, char * musicDirArg) {
+static char * appendSlash(char ** path) {
+	char * temp = *path;
+	int len = strlen(temp);
+
+	if(temp[len-1] != '/') {
+		temp = strdup(*path);
+		free(*path);
+		*path = malloc(len+2);
+		memset(*path, 0, len+2);
+		memcpy(*path, temp, len);
+		(*path)[len] = '/';
+	}
+
+	return * path;
+}
+
+void initPaths() {
+	ConfigParam * musicParam = parseConfigFilePath(CONF_MUSIC_DIR, 1);
+	ConfigParam * playlistParam = parseConfigFilePath(CONF_PLAYLIST_DIR, 1);
+        ConfigParam * fsCharsetParam = getConfigParam(CONF_FS_CHARSET);
+
 	char * charset = NULL;
 	char * originalLocale;
-        struct stat st;
+	DIR * dir;
-        ConfigParam * param;
 
-        playlistDir = prependCwdToPathDup(playlistDirArg);
+	musicDir = appendSlash(&musicParam->value);
-        if((stat(playlistDir,&st))<0) {
+	playlistDir = appendSlash(&playlistParam->value);
-                ERROR("problem stat'ing \"%s\": %s\n", playlistDirArg, strerror(errno));
+
+        if((dir = opendir(playlistDir)) == NULL) {
+                ERROR("cannot open %s \"%s\" (config line %i): %s\n", 
+				CONF_PLAYLIST_DIR, playlistParam->value, 
+				playlistParam->line, strerror(errno));
                 exit(EXIT_FAILURE);
         }
-        if(!S_ISDIR(st.st_mode)) {
+	closedir(dir);
-                ERROR("\"%s\" is not a directory: %s\n", playlistDirArg, strerror(errno));
+
+        if((dir = opendir(musicDir)) == NULL) {
+                ERROR("cannot open %s \"%s\" (config line %i): %s\n", 
+				CONF_MUSIC_DIR, musicParam->value, 
+				musicParam->line, strerror(errno));
                 exit(EXIT_FAILURE);
         }
+	closedir(dir);
 
-        musicDir = prependCwdToPathDup(musicDirArg);
+	if(fsCharsetParam) {
-        if((stat(musicDir,&st))<0) {
+		charset = strdup(fsCharsetParam->value);
-                ERROR("problem stat'ing \"%s\"\n",musicDirArg);
-                exit(EXIT_FAILURE);
-        }
-        if(!S_ISDIR(st.st_mode)) {
-                ERROR("\"%s\" is not a directory\n",musicDirArg);
-                exit(EXIT_FAILURE);
-        }
-
-        param = getConfigParam(CONF_FS_CHARSET);
-
-	if(param) {
-		charset = strdup(param->value);
 	}
 #ifdef HAVE_LOCALE
 #ifdef HAVE_LANGINFO_CODESET

src/path.h

@@ -25,7 +25,7 @@
 
 extern char * musicDir;
 
-void initPaths(char * playlistDirArg, char * musicDirArg);
+void initPaths();
 
 void finishPaths();