From 19f1bfdf40ec7143c7fc3d5d2c330bec8a0bc967 Mon Sep 17 00:00:00 2001 From: Avuton Olrich Date: Wed, 15 Jul 2009 13:36:41 -0700 Subject: [PATCH 01/17] Modify version string to post-release version 0.15.2~git --- NEWS | 3 +++ configure.ac | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index bf9401d6c..d6da68e72 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +ver 0.15.2 (2009/??/??) + + ver 0.15.1 (2009/07/15) * decoders: - flac: fix assertion failure in tag_free() call diff --git a/configure.ac b/configure.ac index 3f631d238..96a890c0e 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ(2.60) -AC_INIT(mpd, 0.15.1, musicpd-dev-team@lists.sourceforge.net) +AC_INIT(mpd, 0.15.2~git, musicpd-dev-team@lists.sourceforge.net) AC_CONFIG_SRCDIR([src/main.c]) AM_INIT_AUTOMAKE([foreign 1.9 dist-bzip2]) AM_CONFIG_HEADER(config.h) From c8c91d9aaab1ea428fa4bafeb72775642e98603a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 17 Jul 2009 17:50:55 +0200 Subject: [PATCH 02/17] configure.ac: fix the --enable-alsa help string --enable means "enable", not "disable". --- configure.ac | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 96a890c0e..43c8d18dc 100644 --- a/configure.ac +++ b/configure.ac @@ -591,8 +591,7 @@ dnl audio output plugins dnl AC_ARG_ENABLE(alsa, - AS_HELP_STRING([--enable-alsa], - [disable ALSA support]),, + AS_HELP_STRING([--enable-alsa], [enable ALSA support]),, enable_alsa=auto) AC_ARG_ENABLE(ao, From a988b9b0259e7d0b1090913087369dd504cd0f45 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 18 Jul 2009 22:45:56 +0200 Subject: [PATCH 03/17] ape: check the tag size (fixes integer underflow) The expression "tagLen - size > 0" may result in an integer underflow and a buffer overflow, when "size" is larger than "tagLen". "size" is read from the input file, and must not be trusted. This patch changes the expression to "tagLen > size", which is a lot safer. --- NEWS | 2 ++ src/tag_ape.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d6da68e72..66ad2cfed 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.15.2 (2009/??/??) +* tags: + - ape: check the tag size (fixes integer underflow) ver 0.15.1 (2009/07/15) diff --git a/src/tag_ape.c b/src/tag_ape.c index d1249fcb2..0d504dc7d 100644 --- a/src/tag_ape.c +++ b/src/tag_ape.c @@ -112,7 +112,7 @@ tag_ape_load(const char *file) /* get the key */ key = p; - while (tagLen - size > 0 && *p != '\0') { + while (tagLen > size && *p != '\0') { p++; tagLen--; } From e3ff0ab6d1f378aec9b98fe930ca42d1f428409e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 19 Jul 2009 17:37:02 +0200 Subject: [PATCH 04/17] tag_ape: removed redundant length check Extend the tagLen check after reading it. Removed the second (redundant) check after the subtraction. --- src/tag_ape.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tag_ape.c b/src/tag_ape.c index 0d504dc7d..ef921141b 100644 --- a/src/tag_ape.c +++ b/src/tag_ape.c @@ -22,6 +22,7 @@ #include +#include #include struct tag * @@ -86,15 +87,15 @@ tag_ape_load(const char *file) /* find beginning of ape tag */ tagLen = GUINT32_FROM_LE(footer.length); - if (tagLen < sizeof(footer)) + if (tagLen <= sizeof(footer) + 10) goto fail; if (fseek(fp, size - tagLen, SEEK_SET)) goto fail; /* read tag into buffer */ tagLen -= sizeof(footer); - if (tagLen <= 0) - goto fail; + assert(tagLen > 10); + buffer = g_malloc(tagLen); if (fread(buffer, 1, tagLen, fp) != tagLen) goto fail; From 0ce727d5d459c2319edc507eb2e71af8a1c9d5dc Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 19 Jul 2009 17:38:46 +0200 Subject: [PATCH 05/17] ape: added protection against large memory allocations The function tag_ape_load() retrieves a 32 bit unsigned integer from the input file, and passes it to g_malloc(). This is dangerous, and may be used for a denial of service attack on MPD. --- NEWS | 1 + src/tag_ape.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 66ad2cfed..8e2c59b78 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.15.2 (2009/??/??) * tags: - ape: check the tag size (fixes integer underflow) + - ape: added protection against large memory allocations ver 0.15.1 (2009/07/15) diff --git a/src/tag_ape.c b/src/tag_ape.c index ef921141b..7cbf32208 100644 --- a/src/tag_ape.c +++ b/src/tag_ape.c @@ -89,6 +89,9 @@ tag_ape_load(const char *file) tagLen = GUINT32_FROM_LE(footer.length); if (tagLen <= sizeof(footer) + 10) goto fail; + if (tagLen > 1024 * 1024) + /* refuse to load more than one megabyte of tag data */ + goto fail; if (fseek(fp, size - tagLen, SEEK_SET)) goto fail; From 322ef3cb805dacff84aea1e18a840e2bbf8cc881 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 12:57:03 +0200 Subject: [PATCH 06/17] mad: skip ID3 frames when libid3tag is disabled When libid3tag is disabled, the libmad decoder plugin is unable to identify ID3 frames. If the file starts with an (unidentified) ID3 frame, it assumes that the file is not a valid MP3 song. This patch solves this by adding minimal stubs for the ID3 functions. --- NEWS | 2 ++ src/decoder/mad_plugin.c | 34 ++++++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 8e2c59b78..674cf61d9 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ ver 0.15.2 (2009/??/??) * tags: - ape: check the tag size (fixes integer underflow) - ape: added protection against large memory allocations +* decoders: + - mad: skip ID3 frames when libid3tag is disabled ver 0.15.1 (2009/07/15) diff --git a/src/decoder/mad_plugin.c b/src/decoder/mad_plugin.c index c6b9d32d3..1ef7183fa 100644 --- a/src/decoder/mad_plugin.c +++ b/src/decoder/mad_plugin.c @@ -350,11 +350,11 @@ parse_id3_replay_gain_info(struct id3_tag *tag) } #endif -#ifdef HAVE_ID3TAG static void mp3_parse_id3(struct mp3_data *data, size_t tagsize, struct tag **mpd_tag, struct replay_gain_info **replay_gain_info_r) { +#ifdef HAVE_ID3TAG struct id3_tag *id3_tag = NULL; id3_length_t count; id3_byte_t const *id3_data; @@ -418,8 +418,34 @@ static void mp3_parse_id3(struct mp3_data *data, size_t tagsize, id3_tag_delete(id3_tag); g_free(allocated); -} +#else /* !HAVE_ID3TAG */ + (void)mpd_tag; + (void)replay_gain_info_r; + + /* This code is enabled when libid3tag is disabled. Instead + of parsing the ID3 frame, it just skips it. */ + + mad_stream_skip(&data->stream, tagsize); #endif +} + +#ifndef HAVE_ID3TAG +/** + * This function emulates libid3tag when it is disabled. Instead of + * doing a real analyzation of the frame, it just checks whether the + * frame begins with the string "ID3". If so, it returns the full + * length. + */ +static signed long +id3_tag_query(const void *p0, size_t length) +{ + const char *p = p0; + + return length > 3 && memcmp(p, "ID3", 3) == 0 + ? length + : 0; +} +#endif /* !HAVE_ID3TAG */ static enum mp3_action decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, @@ -433,7 +459,6 @@ decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, return DECODE_BREAK; } if (mad_header_decode(&data->frame.header, &data->stream)) { -#ifdef HAVE_ID3TAG if ((data->stream).error == MAD_ERROR_LOSTSYNC && (data->stream).this_frame) { signed long tagsize = id3_tag_query((data->stream). @@ -454,7 +479,6 @@ decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, return DECODE_CONT; } } -#endif if (MAD_RECOVERABLE((data->stream).error)) { return DECODE_SKIP; } else { @@ -493,7 +517,6 @@ decodeNextFrame(struct mp3_data *data) return DECODE_BREAK; } if (mad_frame_decode(&data->frame, &data->stream)) { -#ifdef HAVE_ID3TAG if ((data->stream).error == MAD_ERROR_LOSTSYNC) { signed long tagsize = id3_tag_query((data->stream). this_frame, @@ -506,7 +529,6 @@ decodeNextFrame(struct mp3_data *data) return DECODE_CONT; } } -#endif if (MAD_RECOVERABLE((data->stream).error)) { return DECODE_SKIP; } else { From 8e2d9879962775bb29aa4d7556666b2216353bbe Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:31:43 +0200 Subject: [PATCH 07/17] decoder/flac: removed misplaced authorship comment This belongs into "git annotate" or AUTHORS. --- src/decoder/_flac_common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index 713dfe9b2..a6c3a2134 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -67,7 +67,6 @@ flac_find_float_comment(const FLAC__StreamMetadata *block, return false; } -/* replaygain stuff by AliasMrJones */ static void flac_parse_replay_gain(const FLAC__StreamMetadata *block, struct flac_data *data) From cf1fd2b0da4be4675ddb16c5421d949a0e923b20 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:31:46 +0200 Subject: [PATCH 08/17] decoder/flac: return early from flac_find_float_comment() When one metadata check fails, return quickly. This removes 2 levels of indent. --- src/decoder/_flac_common.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index a6c3a2134..a450b6886 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -44,27 +44,28 @@ static bool flac_find_float_comment(const FLAC__StreamMetadata *block, const char *cmnt, float *fl) { - int offset = - FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, cmnt); + int offset; + size_t pos; + int len; + unsigned char tmp, *p; - if (offset >= 0) { - size_t pos = strlen(cmnt) + 1; /* 1 is for '=' */ - int len = block->data.vorbis_comment.comments[offset].length - - pos; - if (len > 0) { - unsigned char tmp; - unsigned char *p = &(block->data.vorbis_comment. - comments[offset].entry[pos]); - tmp = p[len]; - p[len] = '\0'; - *fl = (float)atof((char *)p); - p[len] = tmp; + offset = FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, + cmnt); + if (offset < 0) + return false; - return true; - } - } + pos = strlen(cmnt) + 1; /* 1 is for '=' */ + len = block->data.vorbis_comment.comments[offset].length - pos; + if (len <= 0) + return false; - return false; + p = &block->data.vorbis_comment.comments[offset].entry[pos]; + tmp = p[len]; + p[len] = '\0'; + *fl = (float)atof((char *)p); + p[len] = tmp; + + return true; } static void From 47ed89bd4c6499d475d5f16cb89d7be95763670c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:31:48 +0200 Subject: [PATCH 09/17] decoder/flac: parse all replaygain tags The FLAC replaygain parser used the "||" operator. This made the code stop after the first value which was found. --- NEWS | 1 + src/decoder/_flac_common.c | 32 ++++++++++++++++++-------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index 674cf61d9..374c58d80 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ ver 0.15.2 (2009/??/??) - ape: added protection against large memory allocations * decoders: - mad: skip ID3 frames when libid3tag is disabled + - flac: parse all replaygain tags ver 0.15.1 (2009/07/15) diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index a450b6886..9e2d9f437 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -40,9 +40,9 @@ flac_data_init(struct flac_data *data, struct decoder * decoder, data->tag = NULL; } -static bool +static void flac_find_float_comment(const FLAC__StreamMetadata *block, - const char *cmnt, float *fl) + const char *cmnt, float *fl, bool *found_r) { int offset; size_t pos; @@ -52,12 +52,12 @@ flac_find_float_comment(const FLAC__StreamMetadata *block, offset = FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, cmnt); if (offset < 0) - return false; + return; pos = strlen(cmnt) + 1; /* 1 is for '=' */ len = block->data.vorbis_comment.comments[offset].length - pos; if (len <= 0) - return false; + return; p = &block->data.vorbis_comment.comments[offset].entry[pos]; tmp = p[len]; @@ -65,28 +65,32 @@ flac_find_float_comment(const FLAC__StreamMetadata *block, *fl = (float)atof((char *)p); p[len] = tmp; - return true; + *found_r = true; } static void flac_parse_replay_gain(const FLAC__StreamMetadata *block, struct flac_data *data) { - bool found; + bool found = false; if (data->replay_gain_info) replay_gain_info_free(data->replay_gain_info); data->replay_gain_info = replay_gain_info_new(); - found = flac_find_float_comment(block, "replaygain_album_gain", - &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].gain) || - flac_find_float_comment(block, "replaygain_album_peak", - &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].peak) || - flac_find_float_comment(block, "replaygain_track_gain", - &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].gain) || - flac_find_float_comment(block, "replaygain_track_peak", - &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].peak); + flac_find_float_comment(block, "replaygain_album_gain", + &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].gain, + &found); + flac_find_float_comment(block, "replaygain_album_peak", + &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].peak, + &found); + flac_find_float_comment(block, "replaygain_track_gain", + &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].gain, + &found); + flac_find_float_comment(block, "replaygain_track_peak", + &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].peak, + &found); if (!found) { replay_gain_info_free(data->replay_gain_info); From f4b39bc263b46685742ad120ac9a02d4ba022532 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 22 Jul 2009 13:34:33 +0200 Subject: [PATCH 10/17] decoder/flac: fixed indentation of flac_comment_value() --- src/decoder/_flac_common.c | 40 ++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index 9e2d9f437..ae7d039ce 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -110,25 +110,27 @@ flac_comment_value(const FLAC__StreamMetadata_VorbisComment_Entry *entry, size_t char_tnum_length = 0; const char *comment = (const char*)entry->entry; - if (entry->length > name_length && - g_ascii_strncasecmp(comment, name, name_length) == 0) { - if (char_tnum != NULL) { - char_tnum_length = strlen(char_tnum); - if (entry->length > name_length + char_tnum_length + 2 && - comment[name_length] == '[' && - g_ascii_strncasecmp(comment + name_length + 1, - char_tnum, char_tnum_length) == 0 && - comment[name_length + char_tnum_length + 1] == ']') - name_length = name_length + char_tnum_length + 2; - else if (entry->length > name_length + char_tnum_length && - g_ascii_strncasecmp(comment + name_length, - char_tnum, char_tnum_length) == 0) - name_length = name_length + char_tnum_length; - } - if (comment[name_length] == '=') { - *length_r = entry->length - name_length - 1; - return comment + name_length + 1; - } + if (entry->length <= name_length || + g_ascii_strncasecmp(comment, name, name_length) != 0) + return NULL; + + if (char_tnum != NULL) { + char_tnum_length = strlen(char_tnum); + if (entry->length > name_length + char_tnum_length + 2 && + comment[name_length] == '[' && + g_ascii_strncasecmp(comment + name_length + 1, + char_tnum, char_tnum_length) == 0 && + comment[name_length + char_tnum_length + 1] == ']') + name_length = name_length + char_tnum_length + 2; + else if (entry->length > name_length + char_tnum_length && + g_ascii_strncasecmp(comment + name_length, + char_tnum, char_tnum_length) == 0) + name_length = name_length + char_tnum_length; + } + + if (comment[name_length] == '=') { + *length_r = entry->length - name_length - 1; + return comment + name_length + 1; } return NULL; From e44f31391234607ce0e95d69903142e71d61c813 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:51:35 +0200 Subject: [PATCH 11/17] update: free empty path string (memleak) When you pass an empty string to directory_update_init(), it was not freed by update_task(). --- NEWS | 1 + src/update.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 374c58d80..d312b6d97 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ ver 0.15.2 (2009/??/??) * decoders: - mad: skip ID3 frames when libid3tag is disabled - flac: parse all replaygain tags +* update: free empty path string (memleak) ver 0.15.1 (2009/07/15) diff --git a/src/update.c b/src/update.c index 1088f5338..9a2a2ceb9 100644 --- a/src/update.c +++ b/src/update.c @@ -767,7 +767,6 @@ static void * update_task(void *_path) { if (_path != NULL && !isRootDirectory(_path)) { updatePath((char *)_path); - g_free(_path); } else { struct directory *directory = db_get_root(); struct stat st; @@ -776,6 +775,8 @@ static void * update_task(void *_path) updateDirectory(directory, &st); } + g_free(_path); + if (modified || !db_exists()) db_save(); From 1c4f407a6db4c4795bbbc354f5cf311762fb8e33 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:51:42 +0200 Subject: [PATCH 12/17] decoder/flac: don't allocate cuesheet twice (memleak) The function flac_cue_track() first calls FLAC__metadata_object_new(), then overwrites this pointer with FLAC__metadata_get_cuesheet(). This allocate two FLAC__StreamMetadata objects, but the first pointer is lost, and never freed. --- NEWS | 1 + src/decoder/_flac_common.c | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index d312b6d97..3da38e8d4 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ ver 0.15.2 (2009/??/??) * decoders: - mad: skip ID3 frames when libid3tag is disabled - flac: parse all replaygain tags + - flac: don't allocate cuesheet twice (memleak) * update: free empty path string (memleak) diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index ae7d039ce..e096750f3 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -377,13 +377,15 @@ char* flac_cue_track( const char* pathname, const unsigned int tnum) { - FLAC__StreamMetadata* cs = FLAC__metadata_object_new(FLAC__METADATA_TYPE_CUESHEET); + FLAC__bool success; + FLAC__StreamMetadata* cs; - FLAC__metadata_get_cuesheet(pathname, &cs); - - if (cs == NULL) + success = FLAC__metadata_get_cuesheet(pathname, &cs); + if (!success) return NULL; + assert(cs != NULL); + if (cs->data.cue_sheet.num_tracks <= 1) { FLAC__metadata_object_delete(cs); From 5d6f7803e1059f527a70e541ed3945c19bd78c90 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:51:51 +0200 Subject: [PATCH 13/17] update: free temporary string in container scan (memleak) The return value of map_directory_child_fs() must be freed. --- NEWS | 1 + src/update.c | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 3da38e8d4..68d700bac 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ ver 0.15.2 (2009/??/??) - flac: parse all replaygain tags - flac: don't allocate cuesheet twice (memleak) * update: free empty path string (memleak) +* update: free temporary string in container scan (memleak) ver 0.15.1 (2009/07/15) diff --git a/src/update.c b/src/update.c index 9a2a2ceb9..bdf84ce36 100644 --- a/src/update.c +++ b/src/update.c @@ -430,7 +430,7 @@ update_container_file( struct directory* directory, { char* vtrack = NULL; unsigned int tnum = 0; - const char* pathname = map_directory_child_fs(directory, name); + char* pathname = map_directory_child_fs(directory, name); struct directory* contdir = dirvec_find(&directory->children, name); // directory exists already @@ -446,8 +446,10 @@ update_container_file( struct directory* directory, modified = true; } - else + else { + g_free(pathname); return true; + } } contdir = make_subdir(directory, name); @@ -473,6 +475,8 @@ update_container_file( struct directory* directory, g_free(vtrack); } + g_free(pathname); + if (tnum == 1) { delete_directory(contdir); From 7dddd9beda2bb0505758bb6a32cae6feb3215733 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:52:00 +0200 Subject: [PATCH 14/17] directory: free empty directories after removing them (memleak) dirvec_delete() does not free the object, we have to call directory_free() afterwards. --- NEWS | 1 + src/directory.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 68d700bac..d10ac66e7 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,7 @@ ver 0.15.2 (2009/??/??) - flac: don't allocate cuesheet twice (memleak) * update: free empty path string (memleak) * update: free temporary string in container scan (memleak) +* directory: free empty directories after removing them (memleak) ver 0.15.1 (2009/07/15) diff --git a/src/directory.c b/src/directory.c index 85c24fd04..ef8c038a3 100644 --- a/src/directory.c +++ b/src/directory.c @@ -73,9 +73,14 @@ directory_prune_empty(struct directory *directory) struct dirvec *dv = &directory->children; for (i = dv->nr; --i >= 0; ) { - directory_prune_empty(dv->base[i]); - if (directory_is_empty(dv->base[i])) - dirvec_delete(dv, dv->base[i]); + struct directory *child = dv->base[i]; + + directory_prune_empty(child); + + if (directory_is_empty(child)) { + dirvec_delete(dv, child); + directory_free(child); + } } if (!dv->nr) dirvec_destroy(dv); From 7133f560ec24c90671a40c9f9bc9cea6eb31cc17 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:52:12 +0200 Subject: [PATCH 15/17] output: fixed shout stuck pause bug Explicitly make the output thread leave the ao_pause() loop. This patch is a workaround, and the "pause" flag is not managed in a thread-safe way, but that's good enough for now. --- NEWS | 2 ++ src/output_control.c | 11 +++++++++++ src/output_internal.h | 6 ++++++ src/output_thread.c | 3 +++ 4 files changed, 22 insertions(+) diff --git a/NEWS b/NEWS index d10ac66e7..e0f6a433d 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ ver 0.15.2 (2009/??/??) - mad: skip ID3 frames when libid3tag is disabled - flac: parse all replaygain tags - flac: don't allocate cuesheet twice (memleak) +* output: + - shout: fixed stuck pause bug * update: free empty path string (memleak) * update: free temporary string in container scan (memleak) * directory: free empty directories after removing them (memleak) diff --git a/src/output_control.c b/src/output_control.c index eac9bdfcb..16c0dbb75 100644 --- a/src/output_control.c +++ b/src/output_control.c @@ -76,6 +76,17 @@ audio_output_open(struct audio_output *ao, audio_format_equals(audio_format, &ao->in_audio_format)) { assert(ao->pipe == mp); + if (ao->pause) { + /* unpause with the CANCEL command; this is a + hack, but suits well for forcing the thread + to leave the ao_pause() thread, and we need + to flush the device buffer anyway */ + + /* we're not using audio_output_cancel() here, + because that function is asynchronous */ + ao_command(ao, AO_COMMAND_CANCEL); + } + return true; } diff --git a/src/output_internal.h b/src/output_internal.h index 362d24947..72596c1c3 100644 --- a/src/output_internal.h +++ b/src/output_internal.h @@ -80,6 +80,12 @@ struct audio_output { */ bool open; + /** + * Is the device paused? i.e. the output thread is in the + * ao_pause() loop. + */ + bool pause; + /** * If not NULL, the device has failed, and this timer is used * to estimate how long it should stay disabled (unless diff --git a/src/output_thread.c b/src/output_thread.c index d414ba8d5..785ac808f 100644 --- a/src/output_thread.c +++ b/src/output_thread.c @@ -165,6 +165,7 @@ static void ao_pause(struct audio_output *ao) bool ret; ao_plugin_cancel(ao->plugin, ao->data); + ao->pause = true; ao_command_finished(ao); do { @@ -174,6 +175,8 @@ static void ao_pause(struct audio_output *ao) break; } } while (ao->command == AO_COMMAND_NONE); + + ao->pause = false; } static gpointer audio_output_task(gpointer arg) From f38ce5408b5d0126b8cfe730c91d5203ee59a987 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 14 Aug 2009 11:52:36 +0200 Subject: [PATCH 16/17] output/shout: minimize the unpause latency During the pause loop, manually sleep for 500ms if shout_delay() returns a value greater than that. Don't exhaust libshout's buffer. --- NEWS | 1 + src/output/shout_plugin.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/NEWS b/NEWS index e0f6a433d..4442c70ef 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,7 @@ ver 0.15.2 (2009/??/??) - flac: don't allocate cuesheet twice (memleak) * output: - shout: fixed stuck pause bug + - shout: minimize the unpause latency * update: free empty path string (memleak) * update: free temporary string in container scan (memleak) * directory: free empty directories after removing them (memleak) diff --git a/src/output/shout_plugin.c b/src/output/shout_plugin.c index 8e091679e..4412d26ff 100644 --- a/src/output/shout_plugin.c +++ b/src/output/shout_plugin.c @@ -448,8 +448,15 @@ my_shout_play(void *data, const void *chunk, size_t size, GError **error) static bool my_shout_pause(void *data) { + struct shout_data *sd = (struct shout_data *)data; static const char silence[1020]; + if (shout_delay(sd->shout_conn) > 500) { + /* cap the latency for unpause */ + g_usleep(500000); + return true; + } + return my_shout_play(data, silence, sizeof(silence), NULL); } From 5715534b530cfed0d6650b0fb34cfcb17da4088b Mon Sep 17 00:00:00 2001 From: Avuton Olrich Date: Sat, 15 Aug 2009 11:57:50 -0700 Subject: [PATCH 17/17] mpd version 0.15.2 --- NEWS | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 4442c70ef..d460f0c9d 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.15.2 (2009/??/??) +ver 0.15.2 (2009/08/15) * tags: - ape: check the tag size (fixes integer underflow) - ape: added protection against large memory allocations diff --git a/configure.ac b/configure.ac index 43c8d18dc..b4a5ff529 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ(2.60) -AC_INIT(mpd, 0.15.2~git, musicpd-dev-team@lists.sourceforge.net) +AC_INIT(mpd, 0.15.2, musicpd-dev-team@lists.sourceforge.net) AC_CONFIG_SRCDIR([src/main.c]) AM_INIT_AUTOMAKE([foreign 1.9 dist-bzip2]) AM_CONFIG_HEADER(config.h)