Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#9559 - use BOOTDIR define in the bootloader

Attached to Project: Rockbox
Opened by Thomas Martitz (kugel.) - Monday, 17 November 2008, 09:58 GMT+2
Last edited by Björn Stenberg (zagor) - Sunday, 23 November 2008, 23:08 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

I have no idea why, but the bootloader is hardcoded to /.rockbox (and if that fails to /), even though there's a bootdir define in the target configs. This patch fixes that.
   bootdir-in-bootloader.diff (0.8 KiB) 
 bootloader/common.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

This task depends upon

Closed by  Björn Stenberg (zagor)
Sunday, 23 November 2008, 23:08 GMT+2
Reason for closing:  Accepted
Comment by Thomas Martitz (kugel.) - Monday, 17 November 2008, 17:42 GMT+2
fixed gigabeats. Use bootdir for MI4 format and gigabeat-s as well in some printf's.
Comment by Thomas Martitz (kugel.) - Monday, 17 November 2008, 17:43 GMT+2
forgot to attach
   bootdir-in-bootloaderv2.diff (2.7 KiB) 
 bootloader/main-pp.c         |    4 +++-
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 4 files changed, 9 insertions(+), 6 deletions(-)

Comment by Thomas Martitz (kugel.) - Monday, 17 November 2008, 17:46 GMT+2
fix mi4
   bootdir-in-bootloader-v2b.diff (2.7 KiB) 
 bootloader/main-pp.c         |    4 +++-
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 4 files changed, 9 insertions(+), 6 deletions(-)

Comment by Thomas Martitz (kugel.) - Monday, 17 November 2008, 17:50 GMT+2
fix one more printf
   bootdir-in-bootloader-v2c.diff (3.2 KiB) 
 bootloader/main-pp.c         |    4 +++-
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 bootloader/ipod.c            |    2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

Comment by Thomas Martitz (kugel.) - Monday, 17 November 2008, 17:52 GMT+2
grml, what a mess. Fix mi4 again. That happens if you fix it the first time by only editing the diff and not the actual file :/
   bootdir-in-bootloader-v2d.diff (3.2 KiB) 
 bootloader/main-pp.c         |    4 +++-
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 bootloader/ipod.c            |    2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

Comment by Dominik Riebeling (bluebrother) - Monday, 17 November 2008, 22:43 GMT+2
- printf("Can't load rockbox.ipod:");
+ printf("Can't load %s:", BOOTFILE);

could be done simpler as
printf("Can't load " BOOTFILE ":");
Comment by Thomas Martitz (kugel.) - Monday, 17 November 2008, 23:04 GMT+2
How's that simpler? It's just your preference.
Comment by Dominik Riebeling (bluebrother) - Tuesday, 18 November 2008, 00:28 GMT+2
It doesn't require run-time evaluation and replacement of the format string. It's not just my preference, though it's debatable if it's worth the change (though it's also debatable if moving the string constants to defines is worth it in this limited scope). You could even use puts() with that string instead. Oh, and your version stores two strings, thus two terminating \0 while the concatenated string only stores one.
Comment by Thomas Martitz (kugel.) - Tuesday, 18 November 2008, 00:38 GMT+2
Seems you used the past hours to collect arguments for this text :)

Anyway. I'll change it (if you don't do it, it's one line where you can even edit the diff). You're probably right, doing that at compile time would be better. (and I don't care about either way that much as I already said sometimes).

Still not my definition of simpler. More effective, indeed. But imho not really simpler.
Comment by Thomas Martitz (kugel.) - Tuesday, 18 November 2008, 00:41 GMT+2
BTW: Regarding doing stuff at compile time. Wouldn't it also make sense to remove the filename argument of load_firmware and insert BOOTFILE in those snprintfs too?
Comment by Thomas Martitz (kugel.) - Tuesday, 18 November 2008, 01:03 GMT+2
fix mentioned issues.

What do you think about the BOOTFILE thing?
   bootdir-in-bootloader-v2e.diff (3.2 KiB) 
 bootloader/main-pp.c         |    4 +++-
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 bootloader/ipod.c            |    2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

Comment by Thomas Martitz (kugel.) - Sunday, 23 November 2008, 20:40 GMT+2
final version, only some whitespaces changed
   bootdir-in-bootloader-v3.diff (3.2 KiB) 
 bootloader/main-pp.c         |    4 +++-
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 bootloader/ipod.c            |    2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

Comment by Thomas Martitz (kugel.) - Sunday, 23 November 2008, 20:46 GMT+2
haha ok, one more. Fixed some more printfs using the method bluebrother proposed.
   bootdir-in-bootloader-v3b.diff (3.8 KiB) 
 bootloader/main-pp.c         |    8 +++++---
 bootloader/gigabeat-s.c      |    6 +++---
 bootloader/iaudio_coldfire.c |    2 +-
 bootloader/common.c          |    3 ++-
 bootloader/ipod.c            |    2 +-
 5 files changed, 12 insertions(+), 9 deletions(-)

Loading...