Commit 6459b3ee authored by Eric Wong's avatar Eric Wong

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
parent 4fe965c3
...@@ -260,15 +260,11 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb, ...@@ -260,15 +260,11 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb,
InputStream inStream; InputStream inStream;
InputPlugin *plugin = NULL; InputPlugin *plugin = NULL;
char *path; char *path;
char *relativePath;
if (isRemoteUrl(pc->utf8url)) { if (isRemoteUrl(pc->utf8url))
path = utf8StrToLatin1Dup(pc->utf8url); path = utf8StrToLatin1Dup(pc->utf8url);
} else { else
relativePath = utf8ToFsCharset(pc->utf8url); path = strdup(rmp2amp(utf8ToFsCharset(pc->utf8url)));
path = strdup(rmp2amp(relativePath));
free(relativePath);
}
if (!path) { if (!path) {
dc->error = DECODE_ERROR_FILE; dc->error = DECODE_ERROR_FILE;
......
...@@ -118,7 +118,6 @@ int lsPlaylists(int fd, char *utf8path) ...@@ -118,7 +118,6 @@ int lsPlaylists(int fd, char *utf8path)
int suff; int suff;
if (actlen > MAXPATHLEN - 1 || (dir = opendir(actualPath)) == NULL) { if (actlen > MAXPATHLEN - 1 || (dir = opendir(actualPath)) == NULL) {
free(path);
return 0; return 0;
} }
...@@ -143,7 +142,6 @@ int lsPlaylists(int fd, char *utf8path) ...@@ -143,7 +142,6 @@ int lsPlaylists(int fd, char *utf8path)
dup[suff] = '\0'; dup[suff] = '\0';
if ((utf8 = fsCharsetToUtf8(dup))) { if ((utf8 = fsCharsetToUtf8(dup))) {
insertInList(list, utf8, NULL); insertInList(list, utf8, NULL);
free(utf8);
} }
} }
} }
...@@ -151,7 +149,6 @@ int lsPlaylists(int fd, char *utf8path) ...@@ -151,7 +149,6 @@ int lsPlaylists(int fd, char *utf8path)
} }
closedir(dir); closedir(dir);
free(path);
if (list) { if (list) {
int i; int i;
...@@ -185,16 +182,11 @@ int myStat(char *utf8file, struct stat *st) ...@@ -185,16 +182,11 @@ int myStat(char *utf8file, struct stat *st)
{ {
char *file = utf8ToFsCharset(utf8file); char *file = utf8ToFsCharset(utf8file);
char *actualFile = file; char *actualFile = file;
int ret;
if (actualFile[0] != '/') if (actualFile[0] != '/')
actualFile = rmp2amp(file); actualFile = rmp2amp(file);
ret = stat(actualFile, st); return stat(actualFile, st);
free(file);
return ret;
} }
static int isFile(char *utf8file, time_t * mtime) static int isFile(char *utf8file, time_t * mtime)
......
...@@ -41,18 +41,18 @@ const char *musicDir; ...@@ -41,18 +41,18 @@ const char *musicDir;
static const char *playlistDir; static const char *playlistDir;
static char *fsCharset = NULL; 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) { if (ret)
return convStrDup(str); free(ret);
} return setCharSetConversion(to, from) ? NULL : convStrDup(str);
return NULL;
} }
char *fsCharsetToUtf8(char *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)) { if (ret && !validUtf8String(ret)) {
free(ret); free(ret);
...@@ -64,7 +64,9 @@ char *fsCharsetToUtf8(char *str) ...@@ -64,7 +64,9 @@ char *fsCharsetToUtf8(char *str)
char *utf8ToFsCharset(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) if (!ret)
ret = strdup(str); ret = strdup(str);
......
...@@ -29,8 +29,17 @@ void initPaths(); ...@@ -29,8 +29,17 @@ void initPaths();
void finishPaths(); 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 *utf8ToFsCharset(char *str);
char *fsCharsetToUtf8(char *str); char *fsCharsetToUtf8(char *str);
void setFsCharset(char *charset); void setFsCharset(char *charset);
......
...@@ -1272,8 +1272,6 @@ int deletePlaylist(int fd, char *utf8file) ...@@ -1272,8 +1272,6 @@ int deletePlaylist(int fd, char *utf8file)
strcat(rfile, "."); strcat(rfile, ".");
strcat(rfile, PLAYLIST_FILE_SUFFIX); strcat(rfile, PLAYLIST_FILE_SUFFIX);
free(file);
if ((actualFile = rpp2app(rfile)) && isPlaylist(actualFile)) if ((actualFile = rpp2app(rfile)) && isPlaylist(actualFile))
free(rfile); free(rfile);
else { else {
...@@ -1300,7 +1298,6 @@ int savePlaylist(int fd, char *utf8file) ...@@ -1300,7 +1298,6 @@ int savePlaylist(int fd, char *utf8file)
char *file; char *file;
char *rfile; char *rfile;
char *actualFile; char *actualFile;
char *url;
if (strstr(utf8file, "/")) { if (strstr(utf8file, "/")) {
commandError(fd, ACK_ERROR_ARG, commandError(fd, ACK_ERROR_ARG,
...@@ -1318,8 +1315,6 @@ int savePlaylist(int fd, char *utf8file) ...@@ -1318,8 +1315,6 @@ int savePlaylist(int fd, char *utf8file)
strcat(rfile, "."); strcat(rfile, ".");
strcat(rfile, PLAYLIST_FILE_SUFFIX); strcat(rfile, PLAYLIST_FILE_SUFFIX);
free(file);
actualFile = rpp2app(rfile); actualFile = rpp2app(rfile);
free(rfile); free(rfile);
...@@ -1343,10 +1338,8 @@ int savePlaylist(int fd, char *utf8file) ...@@ -1343,10 +1338,8 @@ int savePlaylist(int fd, char *utf8file)
rmp2amp(utf8ToFsCharset rmp2amp(utf8ToFsCharset
((getSongUrl(playlist.songs[i]))))); ((getSongUrl(playlist.songs[i])))));
} else { } else {
url = utf8ToFsCharset(getSongUrl(playlist.songs[i])); fprintf(fileP, "%s\n",
fprintf(fileP, "%s\n", url); utf8ToFsCharset(getSongUrl(playlist.songs[i])));
free(url);
} }
} }
...@@ -1441,8 +1434,6 @@ static int PlaylistIterFunc(int fd, char *utf8file, ...@@ -1441,8 +1434,6 @@ static int PlaylistIterFunc(int fd, char *utf8file,
strcat(rfile, "."); strcat(rfile, ".");
strcat(rfile, PLAYLIST_FILE_SUFFIX); strcat(rfile, PLAYLIST_FILE_SUFFIX);
free(temp);
if ((actualFile = rpp2app(rfile)) && isPlaylist(actualFile)) if ((actualFile = rpp2app(rfile)) && isPlaylist(actualFile))
free(rfile); free(rfile);
else { else {
...@@ -1499,7 +1490,6 @@ static int PlaylistIterFunc(int fd, char *utf8file, ...@@ -1499,7 +1490,6 @@ static int PlaylistIterFunc(int fd, char *utf8file,
strcpy(s, temp); strcpy(s, temp);
IterFunc(fd, s, &erroredFile); IterFunc(fd, s, &erroredFile);
} }
free(temp);
} else if (slength == MAXPATHLEN) { } else if (slength == MAXPATHLEN) {
s[slength] = '\0'; s[slength] = '\0';
commandError(fd, ACK_ERROR_PLAYLIST_LOAD, commandError(fd, ACK_ERROR_PLAYLIST_LOAD,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment