Make the WPS parser stricter with invalid parameter lists. It will now reject them instead of ignoring them (this includes the second parameter to %m|x|, which is invalid and now causes a failure). Also change the debugging code in order to allow more precise error messages, including the faulty token's index and description. Finally, add a few missing token description and fine-tune the #ifdefs.

git-svn-id: svn://svn.rockbox.org/rockbox/trunk@15624 a1c6a512-1295-4272-9138-f99709370657

3 files changed, 483 insertions, 364 deletions

diff --git a/apps/gui/gwps.h b/apps/gui/gwps.h
index 31f58ee408..56c8a6da6d 100644
--- a/apps/gui/gwps.h
+++ b/apps/gui/gwps.h
@@ -180,9 +180,11 @@ enum wps_token_type {
     WPS_TOKEN_CONDITIONAL_END,
 
     /* Database */
+#ifdef HAVE_TAGCACHE
     WPS_TOKEN_DATABASE_PLAYCOUNT,
     WPS_TOKEN_DATABASE_RATING,
     WPS_TOKEN_DATABASE_AUTOSCORE,
+#endif
 
     /* File */
     WPS_TOKEN_FILE_BITRATE,
@@ -237,7 +239,9 @@ enum wps_token_type {
 
     /* Progressbar */
     WPS_TOKEN_PROGRESSBAR,
+#ifdef HAVE_LCD_CHARCELLS
     WPS_TOKEN_PLAYER_PROGRESSBAR,
+#endif
 
 #ifdef HAVE_LCD_BITMAP
     /* Peakmeter */
diff --git a/apps/gui/wps_debug.c b/apps/gui/wps_debug.c
index 6b7b988321..9443f23730 100644
--- a/apps/gui/wps_debug.c
+++ b/apps/gui/wps_debug.c
@@ -28,9 +28,10 @@
 #include "debug.h"
 #endif
 
-#define PARSE_FAIL_UNCLOSED_COND     1
-#define PARSE_FAIL_INVALID_CHAR      2
-#define PARSE_FAIL_COND_SYNTAX_ERROR 3
+#define PARSE_FAIL_UNCLOSED_COND        1
+#define PARSE_FAIL_INVALID_CHAR         2
+#define PARSE_FAIL_COND_SYNTAX_ERROR    3
+#define PARSE_FAIL_COND_INVALID_PARAM   4
 
 #if defined(SIMULATOR) || defined(__PCTOOL__)
 extern bool debug_wps;
@@ -41,368 +42,431 @@ static char *next_str(bool next) {
     return next ? "next " : "";
 }
 
-static void dump_wps_tokens(struct wps_data *data)
+static char *get_token_desc(struct wps_token *token, struct wps_data *data,
+                            char *buf, int bufsize)
 {
-    struct wps_token *token;
-    int i, j;
-    int indent = 0;
-    char buf[64];
-    bool next;
-    int num_string_tokens = 0;
-
-    /* Dump parsed WPS */
-    for (i = 0, token = data->tokens; i < data->num_tokens; i++, token++) {
-        next = token->next;
-
-        switch(token->type) {
-            case WPS_TOKEN_UNKNOWN:
-                snprintf(buf, sizeof(buf), "Unknown token");
-                break;
-            case WPS_TOKEN_CHARACTER:
-                snprintf(buf, sizeof(buf), "Character '%c'",
-                         token->value.c);
-                break;
+    bool next = token->next;
 
-            case WPS_TOKEN_STRING:
-                snprintf(buf, sizeof(buf), "String '%s'",
-                         data->strings[token->value.i]);
-                num_string_tokens++;
-                break;
+    switch(token->type)
+    {
+        case WPS_NO_TOKEN:
+            snprintf(buf, bufsize, "No token");
+            break;
 
-#ifdef HAVE_LCD_BITMAP
-            case WPS_TOKEN_ALIGN_LEFT:
-                snprintf(buf, sizeof(buf), "align left");
-                break;
+        case WPS_TOKEN_UNKNOWN:
+            snprintf(buf, bufsize, "Unknown token");
+            break;
 
-            case WPS_TOKEN_ALIGN_CENTER:
-                snprintf(buf, sizeof(buf), "align center");
-                break;
+        case WPS_TOKEN_CHARACTER:
+            snprintf(buf, bufsize, "Character '%c'",
+                    token->value.c);
+            break;
 
-            case WPS_TOKEN_ALIGN_RIGHT:
-                snprintf(buf, sizeof(buf), "align right");
-                break;
+        case WPS_TOKEN_STRING:
+            snprintf(buf, bufsize, "String '%s'",
+                     data->strings[token->value.i]);
+            break;
 
-            case WPS_TOKEN_LEFTMARGIN:
-                snprintf(buf, sizeof(buf), "left margin, value: %d",
-                         token->value.i);
-                break;
+#ifdef HAVE_LCD_BITMAP
+        case WPS_TOKEN_ALIGN_LEFT:
+            snprintf(buf, bufsize, "align left");
+            break;
+
+        case WPS_TOKEN_ALIGN_CENTER:
+            snprintf(buf, bufsize, "align center");
+            break;
+
+        case WPS_TOKEN_ALIGN_RIGHT:
+            snprintf(buf, bufsize, "align right");
+            break;
+
+        case WPS_TOKEN_LEFTMARGIN:
+            snprintf(buf, bufsize, "left margin, value: %d",
+                    token->value.i);
+            break;
 #endif
 
-            case WPS_TOKEN_SUBLINE_TIMEOUT:
-                snprintf(buf, sizeof(buf), "subline timeout value: %d",
-                         token->value.i);
-                break;
+        case WPS_TOKEN_SUBLINE_TIMEOUT:
+            snprintf(buf, bufsize, "subline timeout value: %d",
+                    token->value.i);
+            break;
 
-            case WPS_TOKEN_CONDITIONAL:
-                snprintf(buf, sizeof(buf), "conditional, %d options",
-                        token->value.i);
-                break;
+        case WPS_TOKEN_CONDITIONAL:
+            snprintf(buf, bufsize, "conditional, %d options",
+                    token->value.i);
+            break;
 
-            case WPS_TOKEN_CONDITIONAL_START:
-                snprintf(buf, sizeof(buf), "conditional start, next cond: %d",
-                         token->value.i);
-                indent++;
-                break;
+        case WPS_TOKEN_CONDITIONAL_START:
+            snprintf(buf, bufsize, "conditional start, next cond: %d",
+                    token->value.i);
+            break;
 
-            case WPS_TOKEN_CONDITIONAL_OPTION:
-                snprintf(buf, sizeof(buf), "conditional option, next cond: %d",
-                         token->value.i);
-                break;
+        case WPS_TOKEN_CONDITIONAL_OPTION:
+            snprintf(buf, bufsize, "conditional option, next cond: %d",
+                    token->value.i);
+            break;
 
-            case WPS_TOKEN_CONDITIONAL_END:
-                snprintf(buf, sizeof(buf), "conditional end");
-                indent--;
-                break;
+        case WPS_TOKEN_CONDITIONAL_END:
+            snprintf(buf, bufsize, "conditional end");
+            break;
 
 #ifdef HAVE_LCD_BITMAP
-            case WPS_TOKEN_IMAGE_PRELOAD:
-                snprintf(buf, sizeof(buf), "preload image");
-                break;
-
-            case WPS_TOKEN_IMAGE_PRELOAD_DISPLAY:
-                snprintf(buf, sizeof(buf), "display preloaded image %d",
-                         token->value.i);
-                break;
-
-            case WPS_TOKEN_IMAGE_DISPLAY:
-                snprintf(buf, sizeof(buf), "display image");
-                break;
+        case WPS_TOKEN_IMAGE_PRELOAD:
+            snprintf(buf, bufsize, "preload image");
+            break;
+
+        case WPS_TOKEN_IMAGE_PRELOAD_DISPLAY:
+            snprintf(buf, bufsize, "display preloaded image %d",
+                    token->value.i);
+            break;
+
+        case WPS_TOKEN_IMAGE_DISPLAY:
+            snprintf(buf, bufsize, "display image");
+            break;
 #endif
 
 #ifdef HAS_BUTTON_HOLD
-            case WPS_TOKEN_MAIN_HOLD:
-                snprintf(buf, sizeof(buf), "mode hold");
-                break;
+        case WPS_TOKEN_MAIN_HOLD:
+            snprintf(buf, bufsize, "mode hold");
+            break;
 #endif
 
 #ifdef HAS_REMOTE_BUTTON_HOLD
-            case WPS_TOKEN_REMOTE_HOLD:
-                snprintf(buf, sizeof(buf), "mode remote hold");
-                break;
+        case WPS_TOKEN_REMOTE_HOLD:
+            snprintf(buf, bufsize, "mode remote hold");
+            break;
 #endif
 
-            case WPS_TOKEN_REPEAT_MODE:
-                snprintf(buf, sizeof(buf), "mode repeat");
-                break;
-
-            case WPS_TOKEN_PLAYBACK_STATUS:
-                snprintf(buf, sizeof(buf), "mode playback");
-                break;
-
-            case WPS_TOKEN_RTC_DAY_OF_MONTH:
-                snprintf(buf, sizeof(buf), "rtc: day of month (01..31)");
-                break;
-            case WPS_TOKEN_RTC_DAY_OF_MONTH_BLANK_PADDED:
-                snprintf(buf, sizeof(buf),
-                         "rtc: day of month, blank padded ( 1..31)");
-                break;
-            case WPS_TOKEN_RTC_HOUR_24_ZERO_PADDED:
-                snprintf(buf, sizeof(buf), "rtc: hour (00..23)");
-                break;
-            case WPS_TOKEN_RTC_HOUR_24:
-                snprintf(buf, sizeof(buf), "rtc: hour ( 0..23)");
-                break;
-            case WPS_TOKEN_RTC_HOUR_12_ZERO_PADDED:
-                snprintf(buf, sizeof(buf), "rtc: hour (01..12)");
-                break;
-            case WPS_TOKEN_RTC_HOUR_12:
-                snprintf(buf, sizeof(buf), "rtc: hour ( 1..12)");
-                break;
-            case WPS_TOKEN_RTC_MONTH:
-                snprintf(buf, sizeof(buf), "rtc: month (01..12)");
-                break;
-            case WPS_TOKEN_RTC_MINUTE:
-                snprintf(buf, sizeof(buf), "rtc: minute (00..59)");
-                break;
-            case WPS_TOKEN_RTC_SECOND:
-                snprintf(buf, sizeof(buf), "rtc: second (00..59)");
-                break;
-            case WPS_TOKEN_RTC_YEAR_2_DIGITS:
-                snprintf(buf, sizeof(buf),
-                         "rtc: last two digits of year (00..99)");
-                break;
-            case WPS_TOKEN_RTC_YEAR_4_DIGITS:
-                snprintf(buf, sizeof(buf), "rtc: year (1970...)");
-                break;
-            case WPS_TOKEN_RTC_AM_PM_UPPER:
-                snprintf(buf, sizeof(buf),
-                         "rtc: upper case AM or PM indicator");
-                break;
-            case WPS_TOKEN_RTC_AM_PM_LOWER:
-                snprintf(buf, sizeof(buf),
-                         "rtc: lower case am or pm indicator");
-                break;
-            case WPS_TOKEN_RTC_WEEKDAY_NAME:
-                snprintf(buf, sizeof(buf),
-                         "rtc: abbreviated weekday name (Sun..Sat)");
-                break;
-            case WPS_TOKEN_RTC_MONTH_NAME:
-                snprintf(buf, sizeof(buf),
-                         "rtc: abbreviated month name (Jan..Dec)");
-                break;
-            case WPS_TOKEN_RTC_DAY_OF_WEEK_START_MON:
-                snprintf(buf, sizeof(buf),
-                         "rtc: day of week (1..7); 1 is Monday");
-                break;
-            case WPS_TOKEN_RTC_DAY_OF_WEEK_START_SUN:
-                snprintf(buf, sizeof(buf),
-                         "rtc: day of week (0..6); 0 is Sunday");
-                break;
+        case WPS_TOKEN_REPEAT_MODE:
+            snprintf(buf, bufsize, "mode repeat");
+            break;
+
+        case WPS_TOKEN_PLAYBACK_STATUS:
+            snprintf(buf, bufsize, "mode playback");
+            break;
+
+        case WPS_TOKEN_RTC_DAY_OF_MONTH:
+            snprintf(buf, bufsize, "rtc: day of month (01..31)");
+            break;
+        case WPS_TOKEN_RTC_DAY_OF_MONTH_BLANK_PADDED:
+            snprintf(buf, bufsize,
+                    "rtc: day of month, blank padded ( 1..31)");
+            break;
+        case WPS_TOKEN_RTC_HOUR_24_ZERO_PADDED:
+            snprintf(buf, bufsize, "rtc: hour (00..23)");
+            break;
+        case WPS_TOKEN_RTC_HOUR_24:
+            snprintf(buf, bufsize, "rtc: hour ( 0..23)");
+            break;
+        case WPS_TOKEN_RTC_HOUR_12_ZERO_PADDED:
+            snprintf(buf, bufsize, "rtc: hour (01..12)");
+            break;
+        case WPS_TOKEN_RTC_HOUR_12:
+            snprintf(buf, bufsize, "rtc: hour ( 1..12)");
+            break;
+        case WPS_TOKEN_RTC_MONTH:
+            snprintf(buf, bufsize, "rtc: month (01..12)");
+            break;
+        case WPS_TOKEN_RTC_MINUTE:
+            snprintf(buf, bufsize, "rtc: minute (00..59)");
+            break;
+        case WPS_TOKEN_RTC_SECOND:
+            snprintf(buf, bufsize, "rtc: second (00..59)");
+            break;
+        case WPS_TOKEN_RTC_YEAR_2_DIGITS:
+            snprintf(buf, bufsize,
+                    "rtc: last two digits of year (00..99)");
+            break;
+        case WPS_TOKEN_RTC_YEAR_4_DIGITS:
+            snprintf(buf, bufsize, "rtc: year (1970...)");
+            break;
+        case WPS_TOKEN_RTC_AM_PM_UPPER:
+            snprintf(buf, bufsize,
+                    "rtc: upper case AM or PM indicator");
+            break;
+        case WPS_TOKEN_RTC_AM_PM_LOWER:
+            snprintf(buf, bufsize,
+                    "rtc: lower case am or pm indicator");
+            break;
+        case WPS_TOKEN_RTC_WEEKDAY_NAME:
+            snprintf(buf, bufsize,
+                    "rtc: abbreviated weekday name (Sun..Sat)");
+            break;
+        case WPS_TOKEN_RTC_MONTH_NAME:
+            snprintf(buf, bufsize,
+                    "rtc: abbreviated month name (Jan..Dec)");
+            break;
+        case WPS_TOKEN_RTC_DAY_OF_WEEK_START_MON:
+            snprintf(buf, bufsize,
+                    "rtc: day of week (1..7); 1 is Monday");
+            break;
+        case WPS_TOKEN_RTC_DAY_OF_WEEK_START_SUN:
+            snprintf(buf, bufsize,
+                    "rtc: day of week (0..6); 0 is Sunday");
+            break;
 
 #if (CONFIG_CODEC == SWCODEC)
-            case WPS_TOKEN_CROSSFADE:
-                snprintf(buf, sizeof(buf), "crossfade");
-                break;
+        case WPS_TOKEN_CROSSFADE:
+            snprintf(buf, bufsize, "crossfade");
+            break;
 
-            case WPS_TOKEN_REPLAYGAIN:
-                snprintf(buf, sizeof(buf), "replaygain");
-                break;
+        case WPS_TOKEN_REPLAYGAIN:
+            snprintf(buf, bufsize, "replaygain");
+            break;
 #endif
 
 #ifdef HAVE_ALBUMART
-            case WPS_TOKEN_ALBUMART_DISPLAY:
-                snprintf(buf, sizeof(buf), "album art display at x=%d, y=%d, "
-                         "maxwidth=%d, maxheight=%d", data->albumart_x,
-                         data->albumart_y, data->albumart_max_width,
-                         data->albumart_max_height);
-                break;
+        case WPS_TOKEN_ALBUMART_DISPLAY:
+            snprintf(buf, bufsize, "album art display");
+            break;
+
+        case WPS_TOKEN_ALBUMART_FOUND:
+            snprintf(buf, bufsize, "%strack album art conditional",
+                     next_str(next));
+            break;
 #endif
 
 #ifdef HAVE_LCD_BITMAP
-            case WPS_TOKEN_IMAGE_BACKDROP:
-                snprintf(buf, sizeof(buf), "backdrop image");
-                break;
+        case WPS_TOKEN_IMAGE_BACKDROP:
+            snprintf(buf, bufsize, "backdrop image");
+            break;
 
-            case WPS_TOKEN_IMAGE_PROGRESS_BAR:
-                snprintf(buf, sizeof(buf), "progressbar bitmap");
-                break;
+        case WPS_TOKEN_IMAGE_PROGRESS_BAR:
+            snprintf(buf, bufsize, "progressbar bitmap");
+            break;
 
-            case WPS_TOKEN_PEAKMETER:
-                snprintf(buf, sizeof(buf), "peakmeter");
-                break;
+        case WPS_TOKEN_PEAKMETER:
+            snprintf(buf, bufsize, "peakmeter");
+            break;
 #endif
 
-            case WPS_TOKEN_PROGRESSBAR:
-                snprintf(buf, sizeof(buf), "progressbar");
-                break;
+        case WPS_TOKEN_PROGRESSBAR:
+            snprintf(buf, bufsize, "progressbar");
+            break;
 
 #ifdef HAVE_LCD_CHARCELLS
-            case WPS_TOKEN_PLAYER_PROGRESSBAR:
-                snprintf(buf, sizeof(buf), "full line progressbar");
-                break;
+        case WPS_TOKEN_PLAYER_PROGRESSBAR:
+            snprintf(buf, bufsize, "full line progressbar");
+            break;
 #endif
 
-            case WPS_TOKEN_TRACK_TIME_ELAPSED:
-                snprintf(buf, sizeof(buf), "time elapsed in track");
-                break;
-
-            case WPS_TOKEN_TRACK_ELAPSED_PERCENT:
-                snprintf(buf, sizeof(buf), "played percentage of track");
-                break;
-
-            case WPS_TOKEN_PLAYLIST_ENTRIES:
-                snprintf(buf, sizeof(buf), "number of entries in playlist");
-                break;
-
-            case WPS_TOKEN_PLAYLIST_NAME:
-                snprintf(buf, sizeof(buf), "playlist name");
-                break;
-
-            case WPS_TOKEN_PLAYLIST_POSITION:
-                snprintf(buf, sizeof(buf), "position in playlist");
-                break;
-
-            case WPS_TOKEN_TRACK_TIME_REMAINING:
-                snprintf(buf, sizeof(buf), "time remaining in track");
-                break;
-
-            case WPS_TOKEN_PLAYLIST_SHUFFLE:
-                snprintf(buf, sizeof(buf), "playlist shuffle mode");
-                break;
-
-            case WPS_TOKEN_TRACK_LENGTH:
-                snprintf(buf, sizeof(buf), "track length");
-                break;
-
-            case WPS_TOKEN_VOLUME:
-                snprintf(buf, sizeof(buf), "volume");
-                break;
-
-            case WPS_TOKEN_METADATA_ARTIST:
-                snprintf(buf, sizeof(buf), "%strack artist",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_COMPOSER:
-                snprintf(buf, sizeof(buf), "%strack composer",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_ALBUM:
-                snprintf(buf, sizeof(buf), "%strack album",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_GROUPING:
-                snprintf(buf, sizeof(buf), "%strack grouping",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_GENRE:
-                snprintf(buf, sizeof(buf), "%strack genre",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_DISC_NUMBER:
-                snprintf(buf, sizeof(buf), "%strack disc", next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_TRACK_NUMBER:
-                snprintf(buf, sizeof(buf), "%strack number",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_TRACK_TITLE:
-                snprintf(buf, sizeof(buf), "%strack title",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_VERSION:
-                snprintf(buf, sizeof(buf), "%strack ID3 version",
-                         next_str(next));
-                break;
-
-            case WPS_TOKEN_METADATA_YEAR:
-                snprintf(buf, sizeof(buf), "%strack year", next_str(next));
-                break;
-
-            case WPS_TOKEN_BATTERY_PERCENT:
-                snprintf(buf, sizeof(buf), "battery percentage");
-                break;
-
-            case WPS_TOKEN_BATTERY_VOLTS:
-                snprintf(buf, sizeof(buf), "battery voltage");
-                break;
-
-            case WPS_TOKEN_BATTERY_TIME:
-                snprintf(buf, sizeof(buf), "battery time left");
-                break;
-
-            case WPS_TOKEN_BATTERY_CHARGER_CONNECTED:
-                snprintf(buf, sizeof(buf), "battery charger connected");
-                break;
-
-            case WPS_TOKEN_BATTERY_CHARGING:
-                snprintf(buf, sizeof(buf), "battery charging");
-                break;
-
-            case WPS_TOKEN_FILE_BITRATE:
-                snprintf(buf, sizeof(buf), "%sfile bitrate", next_str(next));
-                break;
-
-            case WPS_TOKEN_FILE_CODEC:
-                snprintf(buf, sizeof(buf), "%sfile codec", next_str(next));
-                break;
-
-            case WPS_TOKEN_FILE_FREQUENCY:
-                snprintf(buf, sizeof(buf), "%sfile audio frequency in Hz",
-                         next_str(next));
-                break;
+        case WPS_TOKEN_TRACK_TIME_ELAPSED:
+            snprintf(buf, bufsize, "time elapsed in track");
+            break;
+
+        case WPS_TOKEN_TRACK_ELAPSED_PERCENT:
+            snprintf(buf, bufsize, "played percentage of track");
+            break;
+
+        case WPS_TOKEN_PLAYLIST_ENTRIES:
+            snprintf(buf, bufsize, "number of entries in playlist");
+            break;
+
+        case WPS_TOKEN_PLAYLIST_NAME:
+            snprintf(buf, bufsize, "playlist name");
+            break;
+
+        case WPS_TOKEN_PLAYLIST_POSITION:
+            snprintf(buf, bufsize, "position in playlist");
+            break;
+
+        case WPS_TOKEN_TRACK_TIME_REMAINING:
+            snprintf(buf, bufsize, "time remaining in track");
+            break;
+
+        case WPS_TOKEN_PLAYLIST_SHUFFLE:
+            snprintf(buf, bufsize, "playlist shuffle mode");
+            break;
+
+        case WPS_TOKEN_TRACK_LENGTH:
+            snprintf(buf, bufsize, "track length");
+            break;
+
+        case WPS_TOKEN_VOLUME:
+            snprintf(buf, bufsize, "volume");
+            break;
+
+        case WPS_TOKEN_METADATA_ARTIST:
+            snprintf(buf, bufsize, "%strack artist",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_COMPOSER:
+            snprintf(buf, bufsize, "%strack composer",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_ALBUM:
+            snprintf(buf, bufsize, "%strack album",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_GROUPING:
+            snprintf(buf, bufsize, "%strack grouping",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_GENRE:
+            snprintf(buf, bufsize, "%strack genre",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_DISC_NUMBER:
+            snprintf(buf, bufsize, "%strack disc", next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_TRACK_NUMBER:
+            snprintf(buf, bufsize, "%strack number",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_TRACK_TITLE:
+            snprintf(buf, bufsize, "%strack title",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_VERSION:
+            snprintf(buf, bufsize, "%strack ID3 version",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_ALBUM_ARTIST:
+            snprintf(buf, bufsize, "%strack album artist",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_COMMENT:
+            snprintf(buf, bufsize, "%strack comment",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_METADATA_YEAR:
+            snprintf(buf, bufsize, "%strack year", next_str(next));
+            break;
+
+#ifdef HAVE_TAGCACHE
+        case WPS_TOKEN_DATABASE_PLAYCOUNT:
+            snprintf(buf, bufsize, "track playcount (database)");
+            break;
+
+        case WPS_TOKEN_DATABASE_RATING:
+            snprintf(buf, bufsize, "track rating (database)");
+            break;
+
+        case WPS_TOKEN_DATABASE_AUTOSCORE:
+            snprintf(buf, bufsize, "track autoscore (database)");
+            break;
+#endif
 
-            case WPS_TOKEN_FILE_FREQUENCY_KHZ:
-                snprintf(buf, sizeof(buf), "%sfile audio frequency in KHz",
-                         next_str(next));
-                break;
+        case WPS_TOKEN_BATTERY_PERCENT:
+            snprintf(buf, bufsize, "battery percentage");
+            break;
+
+        case WPS_TOKEN_BATTERY_VOLTS:
+            snprintf(buf, bufsize, "battery voltage");
+            break;
+
+        case WPS_TOKEN_BATTERY_TIME:
+            snprintf(buf, bufsize, "battery time left");
+            break;
+
+        case WPS_TOKEN_BATTERY_CHARGER_CONNECTED:
+            snprintf(buf, bufsize, "battery charger connected");
+            break;
+
+        case WPS_TOKEN_BATTERY_CHARGING:
+            snprintf(buf, bufsize, "battery charging");
+            break;
+
+        case WPS_TOKEN_BATTERY_SLEEPTIME:
+            snprintf(buf, bufsize, "sleep timer");
+            break;
+
+        case WPS_TOKEN_FILE_BITRATE:
+            snprintf(buf, bufsize, "%sfile bitrate", next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_CODEC:
+            snprintf(buf, bufsize, "%sfile codec", next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_FREQUENCY:
+            snprintf(buf, bufsize, "%sfile audio frequency in Hz",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_FREQUENCY_KHZ:
+            snprintf(buf, bufsize, "%sfile audio frequency in KHz",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_NAME:
+            snprintf(buf, bufsize, "%sfile name", next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_NAME_WITH_EXTENSION:
+            snprintf(buf, bufsize, "%sfile name with extension",
+                    next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_PATH:
+            snprintf(buf, bufsize, "%sfile path", next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_SIZE:
+            snprintf(buf, bufsize, "%sfile size", next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_VBR:
+            snprintf(buf, bufsize, "%sfile is vbr", next_str(next));
+            break;
+
+        case WPS_TOKEN_FILE_DIRECTORY:
+            snprintf(buf, bufsize, "%sfile directory, level: %d",
+                    next_str(next), token->value.i);
+            break;
+
+        case WPS_TOKEN_SOUND_PITCH:
+            snprintf(buf, bufsize, "pitch value");
+            break;
+
+        default:
+            snprintf(buf, bufsize, "FIXME (code: %d)",
+                    token->type);
+            break;
+    }
 
-            case WPS_TOKEN_FILE_NAME:
-                snprintf(buf, sizeof(buf), "%sfile name", next_str(next));
-                break;
+    return buf;
+}
 
-            case WPS_TOKEN_FILE_NAME_WITH_EXTENSION:
-                snprintf(buf, sizeof(buf), "%sfile name with extension",
-                         next_str(next));
-                break;
+static void dump_wps_tokens(struct wps_data *data)
+{
+    struct wps_token *token;
+    int i, j;
+    int indent = 0;
+    char buf[64];
+    int num_string_tokens = 0;
 
-            case WPS_TOKEN_FILE_PATH:
-                snprintf(buf, sizeof(buf), "%sfile path", next_str(next));
-                break;
+    /* Dump parsed WPS */
+    for (i = 0, token = data->tokens; i < data->num_tokens; i++, token++)
+    {
+        get_token_desc(token, data, buf, sizeof(buf));
 
-            case WPS_TOKEN_FILE_SIZE:
-                snprintf(buf, sizeof(buf), "%sfile size", next_str(next));
+        switch(token->type)
+        {
+            case WPS_TOKEN_STRING:
+                num_string_tokens++;
                 break;
 
-            case WPS_TOKEN_FILE_VBR:
-                snprintf(buf, sizeof(buf), "%sfile is vbr", next_str(next));
+            case WPS_TOKEN_CONDITIONAL_START:
+                indent++;
                 break;
 
-            case WPS_TOKEN_FILE_DIRECTORY:
-                snprintf(buf, sizeof(buf), "%sfile directory, level: %d",
-                         next_str(next), token->value.i);
+            case WPS_TOKEN_CONDITIONAL_END:
+                indent--;
                 break;
 
             default:
-                snprintf(buf, sizeof(buf), "FIXME (code: %d)",
-                         token->type);
                 break;
         }
 
@@ -528,6 +592,8 @@ void print_debug_info(struct wps_data *data, int fail, int line)
 
     if (fail)
     {
+        char buf[64];
+
         DEBUGF("Failed parsing on line %d : ", line);
         switch (fail)
         {
@@ -536,11 +602,27 @@ void print_debug_info(struct wps_data *data, int fail, int line)
                 break;
 
             case PARSE_FAIL_INVALID_CHAR:
-                DEBUGF("Invalid conditional char (not in an open conditional)");
+                DEBUGF("unexpected conditional char after token %d: \"%s\"",
+                       data->num_tokens-1,
+                       get_token_desc(&data->tokens[data->num_tokens-1], data,
+                                      buf, sizeof(buf))
+                      );
                 break;
 
             case PARSE_FAIL_COND_SYNTAX_ERROR:
-                DEBUGF("Conditional syntax error");
+                DEBUGF("Conditional syntax error after token %d: \"%s\"",
+                       data->num_tokens-1,
+                       get_token_desc(&data->tokens[data->num_tokens-1], data,
+                                      buf, sizeof(buf))
+                      );
+                break;
+
+            case PARSE_FAIL_COND_INVALID_PARAM:
+                DEBUGF("Invalid parameter list for token %d: \"%s\"",
+                       data->num_tokens,
+                       get_token_desc(&data->tokens[data->num_tokens], data,
+                                      buf, sizeof(buf))
+                      );
                 break;
         }
         DEBUGF("\n");
diff --git a/apps/gui/wps_parser.c b/apps/gui/wps_parser.c
index 907a8a3278..07805f20c7 100644
--- a/apps/gui/wps_parser.c
+++ b/apps/gui/wps_parser.c
@@ -42,9 +42,12 @@
 #define WPS_DEFAULTCFG WPS_DIR "/rockbox_default.wps"
 #define RWPS_DEFAULTCFG WPS_DIR "/rockbox_default.rwps"
 
-#define PARSE_FAIL_UNCLOSED_COND     1
-#define PARSE_FAIL_INVALID_CHAR      2
-#define PARSE_FAIL_COND_SYNTAX_ERROR 3
+#define WPS_ERROR_INVALID_PARAM         -1
+
+#define PARSE_FAIL_UNCLOSED_COND        1
+#define PARSE_FAIL_INVALID_CHAR         2
+#define PARSE_FAIL_COND_SYNTAX_ERROR    3
+#define PARSE_FAIL_COND_INVALID_PARAM   4
 
 /* level of current conditional.
    -1 means we're not in a conditional. */
@@ -115,7 +118,7 @@ static int parse_dir_level(const char *wps_bufptr,
         struct wps_token *token, struct wps_data *wps_data);
 
 #ifdef HAVE_LCD_BITMAP
-static int parse_scrollmargin(const char *wps_bufptr,
+static int parse_leftmargin(const char *wps_bufptr,
         struct wps_token *token, struct wps_data *wps_data);
 static int parse_image_special(const char *wps_bufptr,
         struct wps_token *token, struct wps_data *wps_data);
@@ -253,7 +256,7 @@ static const struct wps_tag all_tags[] = {
     { WPS_TOKEN_PLAYBACK_STATUS,          "mp",  WPS_REFRESH_DYNAMIC, NULL },
 
 #ifdef HAVE_LCD_BITMAP
-    { WPS_TOKEN_LEFTMARGIN,               "m",   0, parse_scrollmargin },
+    { WPS_TOKEN_LEFTMARGIN,               "m",   0, parse_leftmargin       },
 #endif
 
 #ifdef HAVE_LCD_BITMAP
@@ -277,9 +280,12 @@ static const struct wps_tag all_tags[] = {
     { WPS_TOKEN_PLAYLIST_NAME,            "pn",  WPS_REFRESH_STATIC,  NULL },
     { WPS_TOKEN_PLAYLIST_SHUFFLE,         "ps",  WPS_REFRESH_DYNAMIC, NULL },
 
+#ifdef HAVE_TAGCACHE
     { WPS_TOKEN_DATABASE_PLAYCOUNT,       "rp",  WPS_REFRESH_DYNAMIC, NULL },
     { WPS_TOKEN_DATABASE_RATING,          "rr",  WPS_REFRESH_DYNAMIC, NULL },
     { WPS_TOKEN_DATABASE_AUTOSCORE,       "ra",  WPS_REFRESH_DYNAMIC, NULL },
+#endif
+
 #if CONFIG_CODEC == SWCODEC
     { WPS_TOKEN_REPLAYGAIN,               "rg",  WPS_REFRESH_STATIC,  NULL },
     { WPS_TOKEN_CROSSFADE,                "xf",  WPS_REFRESH_DYNAMIC, NULL },
@@ -425,8 +431,7 @@ static int parse_image_display(const char *wps_bufptr,
     if (n == -1)
     {
         /* invalid picture display tag */
-        token->type = WPS_TOKEN_UNKNOWN;
-        return 0;
+        return WPS_ERROR_INVALID_PARAM;
     }
 
     token->value.i = n;
@@ -444,15 +449,17 @@ static int parse_image_load(const char *wps_bufptr,
 {
     int n;
     const char *ptr = wps_bufptr;
-    char *pos = NULL;
+    const char *pos = NULL;
+    const char *newline;
 
     /* format: %x|n|filename.bmp|x|y|
        or %xl|n|filename.bmp|x|y|  */
 
     ptr = strchr(ptr, '|') + 1;
     pos = strchr(ptr, '|');
+    newline = strchr(ptr, '\n');
 
-    if (!pos)
+    if (!pos || pos > newline)
         return 0;
 
     /* get the image ID */
@@ -461,8 +468,8 @@ static int parse_image_load(const char *wps_bufptr,
     /* check the image number and load state */
     if(n < 0 || n >= MAX_IMAGES || wps_data->img[n].loaded)
     {
-        /* Skip the rest of the line */
-        return 0;
+        /* Invalid image ID */
+        return WPS_ERROR_INVALID_PARAM;
     }
 
     ptr = pos + 1;
@@ -475,23 +482,23 @@ static int parse_image_load(const char *wps_bufptr,
 
     /* get x-position */
     pos = strchr(ptr, '|');
-    if (pos)
+    if (pos && pos < newline)
         wps_data->img[n].x = atoi(ptr);
     else
     {
         /* weird syntax, bail out */
-        return 0;
+        return WPS_ERROR_INVALID_PARAM;
     }
 
     /* get y-position */
     ptr = pos + 1;
     pos = strchr(ptr, '|');
-    if (pos)
+    if (pos && pos < newline)
         wps_data->img[n].y = atoi(ptr);
     else
     {
         /* weird syntax, bail out */
-        return 0;
+        return WPS_ERROR_INVALID_PARAM;
     }
 
     if (token->type == WPS_TOKEN_IMAGE_DISPLAY)
@@ -505,6 +512,16 @@ static int parse_image_special(const char *wps_bufptr,
                                struct wps_token *token,
                                struct wps_data *wps_data)
 {
+    (void)wps_data; /* kill warning */
+    const char *pos = NULL;
+    const char *newline;
+
+    pos = strchr(wps_bufptr + 1, '|');
+    newline = strchr(wps_bufptr, '\n');
+
+    if (pos > newline)
+        return WPS_ERROR_INVALID_PARAM;
+
     if (token->type == WPS_TOKEN_IMAGE_PROGRESS_BAR)
     {
         /* format: %P|filename.bmp| */
@@ -518,8 +535,6 @@ static int parse_image_special(const char *wps_bufptr,
     }
 #endif
 
-    (void)wps_data; /* to avoid a warning */
-
     /* Skip the rest of the line */
     return skip_end_of_line(wps_bufptr);
 }
@@ -632,7 +647,7 @@ static int parse_albumart_load(const char *wps_bufptr,
                                struct wps_token *token,
                                struct wps_data *wps_data)
 {
-    const char* _pos;
+    const char *_pos, *newline;
     bool parsing;
     const short xalign_mask = WPS_ALBUMART_ALIGN_LEFT |
                               WPS_ALBUMART_ALIGN_CENTER |
@@ -652,25 +667,27 @@ static int parse_albumart_load(const char *wps_bufptr,
 
     /* format: %Cl|x|y|[[l|c|r][d|i|s]mwidth]|[[t|c|b][d|i|s]mheight]| */
 
+    newline = strchr(wps_bufptr, '\n');
+
     /* initial validation and parsing of x and y components */
     if (*wps_bufptr != '|')
-        return 0; /* malformed token: e.g. %Cl7  */
+        return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl7  */
 
     _pos = wps_bufptr + 1;
     if (!isdigit(*_pos))
-        return 0; /* malformed token: e.g. %Cl|@  */
+        return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl|@  */
     wps_data->albumart_x = atoi(_pos);
 
     _pos = strchr(_pos, '|');
-    if (!_pos || !isdigit(*(++_pos)))
-        return 0; /* malformed token: e.g. %Cl|7\n or %Cl|7|@ */
+    if (!_pos || _pos > newline || !isdigit(*(++_pos)))
+        return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl|7\n or %Cl|7|@ */
 
     wps_data->albumart_y = atoi(_pos);
 
     _pos = strchr(_pos, '|');
-    if (!_pos)
-        return 0; /* malformed token: no | after y coordinate
-                     e.g. %Cl|7|59\n */
+    if (!_pos || _pos > newline)
+        return WPS_ERROR_INVALID_PARAM;  /* malformed token: no | after y coordinate
+                                            e.g. %Cl|7|59\n */
 
     /* parsing width field */
     parsing = true;
@@ -721,13 +738,15 @@ static int parse_albumart_load(const char *wps_bufptr,
     /* extract max width data */
     if (*_pos != '|')
     {
-        if (!isdigit(*_pos))
-            return 0; /* malformed token:  e.g. %Cl|7|59|# */
+        if (!isdigit(*_pos))   /* malformed token: e.g. %Cl|7|59|# */
+            return WPS_ERROR_INVALID_PARAM;
+
         wps_data->albumart_max_width = atoi(_pos);
+
         _pos = strchr(_pos, '|');
-        if (!_pos)
-            return 0; /* malformed token: no | after width field
-                         e.g. %Cl|7|59|200\n */
+        if (!_pos || _pos > newline)
+            return WPS_ERROR_INVALID_PARAM; /* malformed token: no | after width field
+                                               e.g. %Cl|7|59|200\n */
     }
 
     /* parsing height field */
@@ -780,12 +799,14 @@ static int parse_albumart_load(const char *wps_bufptr,
     if (*_pos != '|')
     {
         if (!isdigit(*_pos))
-            return 0; /* malformed token  e.g. %Cl|7|59|200|@  */
+            return WPS_ERROR_INVALID_PARAM; /* malformed token  e.g. %Cl|7|59|200|@  */
+
         wps_data->albumart_max_height = atoi(_pos);
+
         _pos = strchr(_pos, '|');
-        if (!_pos)
-            return 0; /* malformed token: no closing |
-                         e.g. %Cl|7|59|200|200\n */
+        if (!_pos || _pos > newline)
+            return WPS_ERROR_INVALID_PARAM; /* malformed token: no closing |
+                                               e.g. %Cl|7|59|200|200\n */
     }
 
     /* if we got here, we parsed everything ok .. ! */
@@ -841,29 +862,29 @@ static int parse_albumart_conditional(const char *wps_bufptr,
 #endif /* HAVE_ALBUMART */
 
 #ifdef HAVE_LCD_BITMAP
-static int parse_scrollmargin(const char *wps_bufptr, struct wps_token *token, 
-                              struct wps_data *wps_data)
+static int parse_leftmargin(const char *wps_bufptr, struct wps_token *token,
+                            struct wps_data *wps_data)
 {
     const char* p;
     const char* pend;
+    const char *newline;
 
     (void)wps_data; /* Kill the warning */
 
-    /* valid tag looks like %m or %m|12|  */
+    /* valid tag looks like %m|12| */
     if(*wps_bufptr == '|')
     {
         p = wps_bufptr + 1;
-
-        if(isdigit(*p) && (pend = strchr(p, '|')))
+        newline = strchr(wps_bufptr, '\n');
+        if(isdigit(*p) && (pend = strchr(p, '|')) && pend < newline)
         {
             token->value.i = atoi(p);
-            return(pend - wps_bufptr + 1);
+            return pend - wps_bufptr + 1;
         }
-    } else {
-        token->value.i = 0;
     }
-            
-    return(0);
+
+    /* invalid tag syntax */
+    return WPS_ERROR_INVALID_PARAM;
 }
 #endif
 
@@ -871,7 +892,7 @@ static int parse_scrollmargin(const char *wps_bufptr, struct wps_token *token,
 /* Parse a generic token from the given string. Return the length read */
 static int parse_token(const char *wps_bufptr, struct wps_data *wps_data)
 {
-    int skip = 0, taglen = 0;
+    int skip = 0, taglen = 0, ret;
     struct wps_token *token = wps_data->tokens + wps_data->num_tokens;
     const struct wps_tag *tag;
 
@@ -898,7 +919,9 @@ static int parse_token(const char *wps_bufptr, struct wps_data *wps_data)
             condindex[level] = wps_data->num_tokens;
             numoptions[level] = 1;
             wps_data->num_tokens++;
-            taglen = 1 + parse_token(wps_bufptr + 1, wps_data);
+            ret = parse_token(wps_bufptr + 1, wps_data);
+            if (ret < 0) return ret;
+            taglen = 1 + ret;
             break;
 
         default:
@@ -914,7 +937,11 @@ static int parse_token(const char *wps_bufptr, struct wps_data *wps_data)
 
             /* if the tag has a special parsing function, we call it */
             if (tag->parse_func)
-                skip += tag->parse_func(wps_bufptr + taglen, token, wps_data);
+            {
+                ret = tag->parse_func(wps_bufptr + taglen, token, wps_data);
+                if (ret < 0) return ret;
+                skip += ret;
+            }
 
             /* Some tags we don't want to save as tokens */
             if (tag->type == WPS_NO_TOKEN)
@@ -945,6 +972,7 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
     char *stringbuf = data->string_buffer;
     int stringbuf_used = 0;
     int fail = 0;
+    int ret;
     line = 1;
     level = -1;
 
@@ -956,7 +984,12 @@ static bool wps_parse(struct wps_data *data, const char *wps_bufptr)
 
             /* Regular tag */
             case '%':
-                wps_bufptr += parse_token(wps_bufptr, data);
+                if ((ret = parse_token(wps_bufptr, data)) < 0)
+                {
+                    fail = PARSE_FAIL_COND_INVALID_PARAM;
+                    break;
+                }
+                wps_bufptr += ret;
                 break;
 
             /* Alternating sublines separator */