FS#5658 - Text viewer hangs when first character is whitespace

Attached to Project: Rockbox
Opened by Mark Arigo (lowlight) - Thursday, 13 July 2006, 18:18 GMT
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


The text viewer will hang if the first character of the text file is whitespace (space, return, tab, etc) and requires a hard reset. This started with the changes made in version 1.37 of plugins/viewer.c and probably only affects HAVE_LCD_BITMAP targets. I've traced the bug:

plugin_start() > viewer_load_settings() > init_need_scrollbar() > viewer_draw(-1) > endless loop at line 698

The bug appears to be because draw_columns is not set until after the call to viewer_draw(-1) in init_need_scrollbar. However, this variable is used in crap_at_width() which is needed in viewer_draw. With draw_columns=0, crop_at_width never advances to the next character and this screws up find_next_line().

As far as I can tell, for a text file with a non-whitespace first character (one that will not hang), viewer_draw(-1) only really "draws" the first character in the text file since crop_at_width() and find_next_line() never advance the string pointer. However, when the first character is whitespace, find_next_line() does advance because of extra checks for whitespace, but crop_at_width() does not. This causes the endless loop at line 698.

The simple solution, that does work, is to set draw_columns in viewer_init():

--- viewer-1.39.c 2006-07-13 11:45:53.566535800 -0400
+++ viewer.c 2006-07-13 13:51:43.129035800 -0400
@@ -942,7 +942,7 @@
pf = rb->font_get(FONT_UI);

display_lines = LCD_HEIGHT / pf->height;
- display_columns = LCD_WIDTH;
+ draw_columns = display_columns = LCD_WIDTH;
/* REAL fixed pitch :) all chars use up 1 cell */
display_lines = 2;

However, since revision 1.11 (over 14 months ago), draw_columns is not set until after viewer_draw(-1) in init_need_scrollbar (for HAVE_LCD_BITMAP targets); so maybe I'm missing the intended purpose of this action. Someone familiar with the viewer.c should review this and determine if this is the proper fix or if the bug lies else where.
This task depends upon

Closed by  Peter D'Hoye (petur)
Saturday, 15 July 2006, 13:54 GMT
Reason for closing:  Accepted
Additional comments about closing:  I\'m not sure either but it fixes the crash and seems to work ok otherwise.
Comment by Mark Arigo (lowlight) - Thursday, 13 July 2006, 19:20 GMT
I just noticed this is also reported in
Comment by Peter D'Hoye (petur) - Thursday, 13 July 2006, 20:44 GMT
even after trying your fix I can still easily crash the viewer.
I think the code needs a more thorough check-up

EDIT: bah, crashing not easily reproducable :(