Rockbox

Tasklist

FS#11899 - logo.c keymap defaults instead of error on undefined targets

Attached to Project: Rockbox
Opened by foolsh (BenjaminBrown) - Saturday, 22 January 2011, 12:07 GMT
Last edited by Dominik Riebeling (bluebrother) - Sunday, 27 February 2011, 16:42 GMT
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.7.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Starting at line 218 logo.c defines keymaps to any target that did not explicitly get defined beforehand. Here is patch that adds defines for the the missing targets and errors out when it gets compiled for an unsupported target just like all the other plugins.
This task depends upon

Closed by  Dominik Riebeling (bluebrother)
Sunday, 27 February 2011, 16:42 GMT
Reason for closing:  Not a Bug
Additional comments about closing:  I read the last comment as request for closure, so closing it. If I'm mistaken on this please request a reopen.
Comment by foolsh (BenjaminBrown) - Saturday, 22 January 2011, 13:02 GMT
I realize that changing such generic catch all code can be fatal I suspect it was left over from some time ago when there were just a few targets. I am half way done testing on all the sdl sim builds, where I can, and have not had any trouble compiling.
Comment by foolsh (BenjaminBrown) - Saturday, 22 January 2011, 14:41 GMT
Here are my test results for this patch. If this is acceptable to any devs, I would like to see this patch committed So I can move ahead dissecting FS#11898 into smaller manageable patches and finally get the plugins compiling nicely for android.
Comment by Thomas Martitz (kugel.) - Saturday, 22 January 2011, 16:00 GMT
Your file mentions that logo.rock doesn't run on the iHP-100/110/115 series. Does it work without your patch?
Comment by foolsh (BenjaminBrown) - Saturday, 22 January 2011, 16:32 GMT
the results show only the simulator build failed but every single other plugin error catches targets without keymaps. Logo wants to assign them arbitrary ones which can cause problems. I will check again just for you.
Comment by foolsh (BenjaminBrown) - Saturday, 22 January 2011, 18:27 GMT
I retested and my mistake it seems to build the simulator quiet nicely for theiHP-100/110/115 and logo is running perfectly well.
Comment by foolsh (BenjaminBrown) - Saturday, 22 January 2011, 19:05 GMT
Looking over everything this change is not mandatory for FS#11898 at all and can be considered solely for the benefit of better code. So sorry for the confusion and extra time I spent on it, ha I'm a dork.
Comment by Thomas Martitz (kugel.) - Saturday, 22 January 2011, 19:11 GMT
Will commit it anyway :)
Comment by Dominik Riebeling (bluebrother) - Sunday, 23 January 2011, 11:18 GMT
My h100 does have logo.rock and it runs (r29079) without any modifications of logo.c. Is this patch only relevant for simulator builds?
Comment by foolsh (BenjaminBrown) - Monday, 24 January 2011, 14:59 GMT
It's relevant to all builds the old code caught every target without a defined keymap, But really considering this patch further I concede that leaving as is will keep it a safe bet come compile time. Close this patch its unneeded.

Loading...