Index: apps/gui/wps_debug.c =================================================================== --- apps/gui/wps_debug.c (Revision 18073) +++ apps/gui/wps_debug.c (Arbeitskopie) @@ -616,7 +616,7 @@ break; case PARSE_FAIL_COND_INVALID_PARAM: - DEBUGF("Invalid parameter list for token %d: \"%s\"", + DEBUGF("Invalid parameter list or value for token %d: \"%s\"", data->num_tokens, get_token_desc(&data->tokens[data->num_tokens], data, buf, sizeof(buf)) Index: apps/gui/gwps.h =================================================================== --- apps/gui/gwps.h (Revision 18073) +++ apps/gui/gwps.h (Arbeitskopie) @@ -126,9 +126,14 @@ (1/HZ sec, or 100ths of sec) */ #define SUBLINE_RESET -1 +/* Status values of WPS parsing. Error values must be negative! */ enum wps_parse_error { - PARSE_OK, - PARSE_FAIL_UNCLOSED_COND, + PARSE_OK = 0, + + /* Assign a large enough negative number. Subsequent symbols */ + /* will get values incremented by one. As long as we have less */ + /* than 100 error codes everything is OK. */ + PARSE_FAIL_UNCLOSED_COND = -99, PARSE_FAIL_INVALID_CHAR, PARSE_FAIL_COND_SYNTAX_ERROR, PARSE_FAIL_COND_INVALID_PARAM, Index: apps/gui/wps_parser.c =================================================================== --- apps/gui/wps_parser.c (Revision 18073) +++ apps/gui/wps_parser.c (Arbeitskopie) @@ -57,8 +57,6 @@ #define WPS_DEFAULTCFG WPS_DIR "/rockbox_default.wps" #define RWPS_DEFAULTCFG WPS_DIR "/rockbox_default.rwps" -#define WPS_ERROR_INVALID_PARAM -1 - /* level of current conditional. -1 means we're not in a conditional. */ static int level = -1; @@ -103,11 +101,12 @@ function is called, the token type has already been set. The function must fill in the details and possibly add more tokens to the token array. It should return the number of chars that - has been consumed. + has been consumed (>=0) or a negative value in case of an error. wps_bufptr points to the char following the tag (i.e. where details begin). - token is the pointer to the 'main' token being parsed + token is the pointer to the 'main' token being parsed. Its type + is already set. */ typedef int (*wps_tag_parse_func)(const char *wps_bufptr, struct wps_token *token, struct wps_data *wps_data); @@ -457,14 +456,14 @@ if (n == -1) { /* invalid picture display tag */ - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } if ((subimage = get_image_id(wps_bufptr[1])) != -1) { /* Sanity check */ if (subimage >= wps_data->img[n].num_subimages) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; /* Store sub-image number to display in high bits */ token->value.i = n | (subimage << 8); @@ -493,16 +492,16 @@ */ if (*ptr != '|') - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; ptr++; if (!(ptr = parse_list("ssdd", NULL, '|', ptr, &id, &filename, &x, &y))) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; /* Check there is a terminating | */ if (*ptr != '|') - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; /* get the image ID */ n = get_image_id(*id); @@ -511,7 +510,7 @@ if(n < 0 || n >= MAX_IMAGES || wps_data->img[n].loaded) { /* Invalid image ID */ - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } /* save a pointer to the filename */ @@ -537,7 +536,7 @@ wps_data->img[n].num_subimages = atoi(ptr); if (wps_data->img[n].num_subimages <= 0) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } /* Skip the rest of the line */ @@ -554,7 +553,7 @@ if (letter < 'a' || letter > 'z') { /* invalid viewport tag */ - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } token->value.i = letter; return 1; @@ -588,7 +587,7 @@ #endif if (wps_data->num_viewports >= WPS_MAX_VIEWPORTS) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_LIMITS_EXCEEDED; wps_data->num_viewports++; /* check for the optional letter to signify its a hideable viewport */ @@ -606,12 +605,12 @@ wps_data->viewports[wps_data->num_viewports].label = label; } else - return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl7 */ + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token: e.g. %Cl7 */ ptr += 3; } } if (*ptr != '|') - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; ptr++; vp = &wps_data->viewports[wps_data->num_viewports].vp; @@ -632,7 +631,7 @@ { if (!(ptr = parse_list("dddddcc", &set, '|', ptr, &vp->x, &vp->y, &vp->width, &vp->height, &vp->font, &vp->fg_pattern,&vp->bg_pattern))) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } else #endif @@ -643,7 +642,7 @@ vp->bg_pattern = 3; if (!(ptr = parse_list("dddddgg", &set, '|', ptr, &vp->x, &vp->y, &vp->width, &vp->height, &vp->font, &vp->fg_pattern, &vp->bg_pattern))) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } else #endif @@ -652,7 +651,7 @@ { if (!(ptr = parse_list("ddddd", &set, '|', ptr, &vp->x, &vp->y, &vp->width, &vp->height, &vp->font))) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } else #endif @@ -660,10 +659,10 @@ /* Check for trailing | */ if (*ptr != '|') - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; if (!LIST_VALUE_PARSED(set, PL_X) || !LIST_VALUE_PARSED(set, PL_Y)) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; /* fix defaults */ if (!LIST_VALUE_PARSED(set, PL_WIDTH)) @@ -683,7 +682,7 @@ (vp->y >= lcd_height) || ((vp->y + vp->height) > lcd_height)) { - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; } #ifdef HAVE_LCD_COLOR @@ -728,7 +727,7 @@ newline = strchr(wps_bufptr, '\n'); if (pos > newline) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; #if LCD_DEPTH > 1 if (token->type == WPS_TOKEN_IMAGE_BACKDROP) { @@ -824,7 +823,7 @@ wps_data->viewports[wps_data->num_viewports].first_line); if (wps_data->progressbar_count >= MAX_PROGRESSBARS) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; pb = &wps_data->progressbar[wps_data->progressbar_count]; pb->have_bitmap_pb = false; @@ -844,7 +843,7 @@ if (!(ptr = parse_list("sdddd", &set, '|', ptr, &filename, &x, &y, &width, &height))) - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; if (LIST_VALUE_PARSED(set, PB_FILENAME)) /* filename */ bmp_names[PROGRESSBAR_BMP+wps_data->progressbar_count] = filename; if (LIST_VALUE_PARSED(set, PB_X)) /* x */ @@ -908,23 +907,25 @@ /* initial validation and parsing of x and y components */ if (*wps_bufptr != '|') - return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl7 */ + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token: e.g. %Cl7 */ _pos = wps_bufptr + 1; if (!isdigit(*_pos)) - return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl|@ */ + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token: e.g. %Cl|@ */ wps_data->albumart_x = atoi(_pos); _pos = strchr(_pos, '|'); if (!_pos || _pos > newline || !isdigit(*(++_pos))) - return WPS_ERROR_INVALID_PARAM; /* malformed token: e.g. %Cl|7\n or %Cl|7|@ */ + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token: e.g. %Cl|7\n or %Cl|7|@ */ wps_data->albumart_y = atoi(_pos); _pos = strchr(_pos, '|'); if (!_pos || _pos > newline) - return WPS_ERROR_INVALID_PARAM; /* malformed token: no | after y coordinate - e.g. %Cl|7|59\n */ + { + /* malformed token: no | after y coordinate e.g. %Cl|7|59\n */ + return PARSE_FAIL_COND_INVALID_PARAM; + } /* parsing width field */ parsing = true; @@ -976,13 +977,13 @@ if (*_pos != '|') { if (!isdigit(*_pos)) /* malformed token: e.g. %Cl|7|59|# */ - return WPS_ERROR_INVALID_PARAM; + return PARSE_FAIL_COND_INVALID_PARAM; wps_data->albumart_max_width = atoi(_pos); _pos = strchr(_pos, '|'); if (!_pos || _pos > newline) - return WPS_ERROR_INVALID_PARAM; /* malformed token: no | after width field + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token: no | after width field e.g. %Cl|7|59|200\n */ } @@ -1036,13 +1037,13 @@ if (*_pos != '|') { if (!isdigit(*_pos)) - return WPS_ERROR_INVALID_PARAM; /* malformed token e.g. %Cl|7|59|200|@ */ + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token e.g. %Cl|7|59|200|@ */ wps_data->albumart_max_height = atoi(_pos); _pos = strchr(_pos, '|'); if (!_pos || _pos > newline) - return WPS_ERROR_INVALID_PARAM; /* malformed token: no closing | + return PARSE_FAIL_COND_INVALID_PARAM; /* malformed token: no closing | e.g. %Cl|7|59|200|200\n */ } @@ -1099,7 +1100,11 @@ #endif /* HAVE_ALBUMART */ -/* Parse a generic token from the given string. Return the length read */ +/* + * Parse a generic token from the given string. Return the number of + * characters read (>=0) or a negative number in case of an error + * (bad syntax etc.) + */ static int parse_token(const char *wps_bufptr, struct wps_data *wps_data) { int skip = 0, taglen = 0, ret; @@ -1170,10 +1175,14 @@ return skip; } -/* Parses the WPS. - data is the pointer to the structure where the parsed WPS should be stored. - It is initialised. - wps_bufptr points to the string containing the WPS tags */ +/* + * Parses the WPS. + * data is the pointer to the already initialised structure where the + * parsed WPS should be stored. + * wps_bufptr points to the string containing the WPS tags + * + * Returns true iff the WPS was successfully parsed + */ static bool wps_parse(struct wps_data *data, const char *wps_bufptr) { if (!data || !wps_bufptr || !*wps_bufptr) @@ -1186,7 +1195,8 @@ line = 1; level = -1; - while(*wps_bufptr && !fail && data->num_tokens < WPS_MAX_TOKENS - 1 + while(*wps_bufptr && (fail == PARSE_OK) + && data->num_tokens < WPS_MAX_TOKENS - 1 && data->num_viewports < WPS_MAX_VIEWPORTS && data->num_lines < WPS_MAX_LINES) { @@ -1198,14 +1208,15 @@ if ((ret = parse_token(wps_bufptr, data)) < 0) { fail = PARSE_FAIL_COND_INVALID_PARAM; - break; } else if (level >= WPS_MAX_COND_LEVEL - 1) { fail = PARSE_FAIL_LIMITS_EXCEEDED; - break; } - wps_bufptr += ret; + else + { + wps_bufptr += ret; + } break; /* Alternating sublines separator */ @@ -1377,23 +1388,28 @@ } } - if (!fail && level >= 0) /* there are unclosed conditionals */ - fail = PARSE_FAIL_UNCLOSED_COND; - - if (*wps_bufptr && !fail) - /* one of the limits of the while loop was exceeded */ - fail = PARSE_FAIL_LIMITS_EXCEEDED; + if (fail == PARSE_OK) + { + if (level >= 0) /* there are unclosed conditionals */ + fail = PARSE_FAIL_UNCLOSED_COND; + else if (*wps_bufptr) + /* one of the limits of the while loop was exceeded */ + fail = PARSE_FAIL_LIMITS_EXCEEDED; + else + { + data->viewports[data->num_viewports].last_line = data->num_lines - 1; - data->viewports[data->num_viewports].last_line = data->num_lines - 1; + /* We have finished with the last viewport, so increment count */ + data->num_viewports++; + } + } - /* We have finished with the last viewport, so increment count */ - data->num_viewports++; #ifdef DEBUG print_debug_info(data, fail, line); #endif - return (fail == 0); + return (fail == PARSE_OK); } #ifdef HAVE_LCD_BITMAP