Revert leaks from r4311, and also the leak fixes as a result of that

utf8ToFsCharset() and fsCharsetToUtf8() got very broken in r4311, and
resulted in several commits to fix those leaks.  Unfortunately, not all
of those newly introduced leaks were fixed, nor was the result pretty.

Also, fixed a double-free in lsPlaylists().  This is very hard
to trigger (and therefore exploit) at the moment because we
check printDirectoryInfo() beforehand.

Intended behavior for utf8ToFsCharset() and fsCharsetToUtf8() as
God^H^H^Hshank originally intended is now documented in path.h
to prevent future errors like this.

mpd could still use some good valgrind testing before the 0.12.0
release.

<plug>In addition to reducing heap fragmentation, malloc
reductions from mpd-ke greatly reduces the chance of leaks from
happening due to programming errors.</plug>

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

5 files changed, 26 insertions, 37 deletions

diff --git a/src/decode.c b/src/decode.c
index c07da9907..158727f6f 100644
--- a/src/decode.c
+++ b/src/decode.c
@@ -260,15 +260,11 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb,
 	InputStream inStream;
 	InputPlugin *plugin = NULL;
 	char *path;
-	char *relativePath;
 
-	if (isRemoteUrl(pc->utf8url)) {
+	if (isRemoteUrl(pc->utf8url))
 		path = utf8StrToLatin1Dup(pc->utf8url);
-	} else {
-		relativePath = utf8ToFsCharset(pc->utf8url);
-		path = strdup(rmp2amp(relativePath));
-		free(relativePath);
-	}
+	else
+		path = strdup(rmp2amp(utf8ToFsCharset(pc->utf8url)));
 
 	if (!path) {
 		dc->error = DECODE_ERROR_FILE;
diff --git a/src/ls.c b/src/ls.c
index 376ebb058..427d01b43 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -118,7 +118,6 @@ int lsPlaylists(int fd, char *utf8path)
 	int suff;
 
 	if (actlen > MAXPATHLEN - 1 || (dir = opendir(actualPath)) == NULL) {
-		free(path);
 		return 0;
 	}
 
@@ -143,7 +142,6 @@ int lsPlaylists(int fd, char *utf8path)
 					dup[suff] = '\0';
 					if ((utf8 = fsCharsetToUtf8(dup))) {
 						insertInList(list, utf8, NULL);
-						free(utf8);
 					}
 				}
 			}
@@ -151,7 +149,6 @@ int lsPlaylists(int fd, char *utf8path)
 	}
 
 	closedir(dir);
-	free(path);
 
 	if (list) {
 		int i;
@@ -185,16 +182,11 @@ int myStat(char *utf8file, struct stat *st)
 {
 	char *file = utf8ToFsCharset(utf8file);
 	char *actualFile = file;
-	int ret;
 
 	if (actualFile[0] != '/')
 		actualFile = rmp2amp(file);
 
-	ret = stat(actualFile, st);
-
-	free(file);
-
-	return ret;
+	return stat(actualFile, st);
 }
 
 static int isFile(char *utf8file, time_t * mtime)
diff --git a/src/path.c b/src/path.c
index 69c86391b..b05843514 100644
--- a/src/path.c
+++ b/src/path.c
@@ -41,18 +41,18 @@ const char *musicDir;
 static const char *playlistDir;
 static char *fsCharset = NULL;
 
-static char *pathConvCharset(char *to, char *from, char *str)
+static char *pathConvCharset(char *to, char *from, char *str, char *ret)
 {
-	if (setCharSetConversion(to, from) == 0) {
-		return convStrDup(str);
-	}
-
-	return NULL;
+	if (ret)
+		free(ret);
+	return setCharSetConversion(to, from) ? NULL : convStrDup(str);
 }
 
 char *fsCharsetToUtf8(char *str)
 {
-	char *ret = pathConvCharset("UTF-8", fsCharset, str);
+	static char *ret = NULL;
+
+	ret = pathConvCharset("UTF-8", fsCharset, str, ret);
 
 	if (ret && !validUtf8String(ret)) {
 		free(ret);
@@ -64,7 +64,9 @@ char *fsCharsetToUtf8(char *str)
 
 char *utf8ToFsCharset(char *str)
 {
-	char *ret = pathConvCharset(fsCharset, "UTF-8", str);
+	static char *ret = NULL;
+
+	ret = pathConvCharset(fsCharset, "UTF-8", str, ret);
 
 	if (!ret)
 		ret = strdup(str);
diff --git a/src/path.h b/src/path.h
index 222e145a5..e9fd36fa0 100644
--- a/src/path.h
+++ b/src/path.h
@@ -29,8 +29,17 @@ void initPaths();
 
 void finishPaths();
 
+/* utf8ToFsCharset() and fsCharsetToUtf8()
+ * Each returns a static pointer to a dynamically allocated buffer
+ * which means:
+ * - Do not manually free the return value of these functions, it'll be
+ *   automatically freed the next time it is called.
+ * - They are not reentrant, strdup the return value immediately if
+ *   you expect to call one of these functions again, but still need the
+ *   previous result.
+ * - The static pointer is unique to each function.
+ */
 char *utf8ToFsCharset(char *str);
-
 char *fsCharsetToUtf8(char *str);
 
 void setFsCharset(char *charset);
diff --git a/src/playlist.c b/src/playlist.c
index 359e3ac24..80c50f5b5 100644
--- a/src/playlist.c
+++ b/src/playlist.c
@@ -1272,8 +1272,6 @@ int deletePlaylist(int fd, char *utf8file)
 	strcat(rfile, ".");
 	strcat(rfile, PLAYLIST_FILE_SUFFIX);
 
-	free(file);
-
 	if ((actualFile = rpp2app(rfile)) && isPlaylist(actualFile))
 		free(rfile);
 	else {
@@ -1300,7 +1298,6 @@ int savePlaylist(int fd, char *utf8file)
 	char *file;
 	char *rfile;
 	char *actualFile;
-	char *url;
 
 	if (strstr(utf8file, "/")) {
 		commandError(fd, ACK_ERROR_ARG,
@@ -1318,8 +1315,6 @@ int savePlaylist(int fd, char *utf8file)
 	strcat(rfile, ".");
 	strcat(rfile, PLAYLIST_FILE_SUFFIX);
 
-	free(file);
-
 	actualFile = rpp2app(rfile);
 
 	free(rfile);
@@ -1343,10 +1338,8 @@ int savePlaylist(int fd, char *utf8file)
 				  rmp2amp(utf8ToFsCharset
 					  ((getSongUrl(playlist.songs[i])))));
 		} else {
-			url = utf8ToFsCharset(getSongUrl(playlist.songs[i]));
-			fprintf(fileP, "%s\n", url);
-			free(url);
-
+			fprintf(fileP, "%s\n",
+			        utf8ToFsCharset(getSongUrl(playlist.songs[i])));
 		}
 	}
 
@@ -1441,8 +1434,6 @@ static int PlaylistIterFunc(int fd, char *utf8file,
 	strcat(rfile, ".");
 	strcat(rfile, PLAYLIST_FILE_SUFFIX);
 
-	free(temp);
-
 	if ((actualFile = rpp2app(rfile)) && isPlaylist(actualFile))
 		free(rfile);
 	else {
@@ -1499,7 +1490,6 @@ static int PlaylistIterFunc(int fd, char *utf8file,
 				strcpy(s, temp);
 				IterFunc(fd, s, &erroredFile);
 			}
-			free(temp);
 		} else if (slength == MAXPATHLEN) {
 			s[slength] = '\0';
 			commandError(fd, ACK_ERROR_PLAYLIST_LOAD,