Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#8827 - ipodpatcher errorlevels fix

Attached to Project: Rockbox
Opened by Keripo (Keripo) - Saturday, 29 March 2008, 21:43 GMT+2
Last edited by Dominik Riebeling (bluebrother) - Sunday, 05 June 2011, 13:03 GMT+2
Task Type Patches
Category Bootloader
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

At the moment, ipodpatcher doesn't seem to have any standard errorlevels/exit values. In some cases, both success and failure return the exit value of '0'. Some situations even return arbitrary values of '4' or '5'. I redid main.c for ipodpatcher such that it returned -1 if there were iPod detection/access issues, 0 if the chosen action was successful, and 1 if unsuccessful. A "switch/case" was used instead of the plethora of "if/else" statements such that the code is easier to follow and redundant code was reordered. Strings containing "ipod", etc. were also corrected (e.g. to "iPod" and "iTunes") so I've also included a small patch for ipodpatcher.c. The flow of the program and all the actions are still the same (so it will still work exactly the same as before); things have just been re-ordered a bit and cleaned up with proper errorlevels/exit values.
   main.c (16.2 KiB)
   ipodpatcher.c.patch (2.5 KiB) 
 ipodpatcher.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

This task depends upon

Closed by  Dominik Riebeling (bluebrother)
Sunday, 05 June 2011, 13:03 GMT+2
Reason for closing:  Rejected
Additional comments about closing:  no real name
Comment by Keripo (Keripo) - Saturday, 29 March 2008, 21:49 GMT+2
For more colourful versions:
main.c: http://pastebin.ca/962459
ipodpatcher.c.patch: http://pastebin.ca/962513
Comment by Dominik Riebeling (bluebrother) - Saturday, 29 March 2008, 21:52 GMT+2
Your main.c contains tabs and it would've been nicer to have it as diff for easier tracking of the relevant changes.

About your renaming of ipod -> iPod etc: there is currently some mixture about the writing of Ipod around. The manual uses Ipod, the website iPod, ipodpatcher uses ipod. IMO we should be consistent with the naming, but until we have agreed on a specific way of writing renaming isn't a good idea. Feel free to join the discussion.
Comment by Keripo (Keripo) - Saturday, 29 March 2008, 22:54 GMT+2
There's the diff with spaces instead of tabs.

As for the renaming of ipod->iPod, etc., if the manual uses "Ipod", etc., I suppose it'd be better to keep it how it is right now (i.e. ignore ipodpatcher.c.patch). My opinion is that the canonic name ("iPod" and "iTunes") should be used to avoid confusion for new users, but if it has already been set in stone like such, not worth the effort of changing everything.
   main.c.patch (10.4 KiB) 
 main.c |  144 +++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 74 insertions(+), 70 deletions(-)

Comment by Keripo (Keripo) - Saturday, 29 March 2008, 23:00 GMT+2
Er, fixed syntax error.
   main.c.patch (10.3 KiB) 
 main.c |  142 ++++++++++++++++++++++++++++++++---------------------------------
 1 file changed, 72 insertions(+), 70 deletions(-)

Comment by Dominik Riebeling (bluebrother) - Saturday, 29 March 2008, 23:34 GMT+2
The naming hasn't been "set in stone" -- it was just agreed to use Ipod etc. for the manual some while back. Recently the discussion came up if that shouldn't be changed back to iPod. While I got used to Ipod I have no strong opinion for (or against) that capitalization, I rather have a strong concern that we should be consistent.

Feel free to join on IRC and discuss the issue.

Loading...