FS#6189 - 'Out of menus' error check

Attached to Project: Rockbox
Opened by absurdlyobfuscated (DrSpud) - Sunday, 15 October 2006, 06:13 GMT
Last edited by Jonathan Gordon (jdgordon) - Saturday, 16 June 2007, 13:00 GMT
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Both menu_run and menu_exit don't check to see if they have been passed a valid menu slot. This causes a freeze/segfault if menu_find_free fails in an 'Out of menus' condition and no error checking is done on the result. This simple patch adds checks, though we'd probably want a visual error message displayed somewhere too.
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Saturday, 16 June 2007, 13:00 GMT
Reason for closing:  Out of Date
Comment by absurdlyobfuscated (DrSpud) - Sunday, 15 October 2006, 06:53 GMT
Forgot about menu_show...
Now prevents  FS#6010  from crashing :)
Comment by Peter D'Hoye (petur) - Sunday, 10 December 2006, 22:26 GMT
no it doesn't

It does some really weird things if you try to reproduce  FS#6010 .
First hit the limit of 6 iterations, then press left to back out. Watch the screen go crazy.
Pressing left enough times eventually freezes my player.
Comment by absurdlyobfuscated (DrSpud) - Tuesday, 12 December 2006, 05:59 GMT
WFM on iRiver H1x0. You were using 'out_of_menus_error_check2.patch', right? I know the first one didn't fix all the problems from an out of menus condition.

I suppose this patch isn't as necessary now (as long we make sure MAX_MENUS is big enough), since your patch fixed the cause that made the lack of checks a problem (thanks a lot for that, by the way). It doesn't add much to the code size, however...
Comment by Peter D'Hoye (petur) - Tuesday, 12 December 2006, 07:18 GMT
yes, I tried out_of_menus_error_check2.patch with  FS#6010  and it didn't fix that. It didn't crash due to too many recursions but just having one recursion was already enough to cause some internal corruption when going back down the call list (by pressing left)

Checking the return value is needed too with an error message, which is why I didn't close this entry yet ;)