Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#11575 - RaaA should not be storing the real full path in config.cfg

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Sunday, 29 August 2010, 04:41 GMT+2
Last edited by Jonathan Gordon (jdgordon) - Tuesday, 07 December 2010, 01:29 GMT+2
Task Type Bugs
Category Settings
Status Closed
Assigned To Thomas Martitz (kugel.)
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

from my config.cfg
"
playlist catalog directory: /usr/local/share/rockbox/Playlis
"
/usr/local/share/rockbox should be added at runtime, and stored speratly or the actual path is cut off
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Tuesday, 07 December 2010, 01:29 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  r28752
Comment by Nils Wallménius (nls) - Sunday, 29 August 2010, 08:55 GMT+2
is there any reason why we limit this path to 32 chars except to save some memory?
Comment by Jonathan Gordon (jdgordon) - Sunday, 29 August 2010, 09:09 GMT+2
nope, but then there is another more serious problem which wasnt obvious when I filed this... the Playlists directory should be in the user writable area so it should be ~/.config/rockbox.org/Playlists by default.
Comment by Thomas Martitz (kugel.) - Sunday, 29 August 2010, 12:18 GMT+2
I think it actually is, it's supposed to be anyways.

"/usr/local/share/rockbox/xxx" is translated into $HOME/.config/rockbox.org/xxx (if it exists) at even though it says /usr/local/share.

The path doesn't mean much anyway. Only the the basename of the file (without path and extensions) is actually stored in RAM. From there on it looks in $HOME/.config/rockbox.org/xxx, then /usr/local/share/rockbox for the files/folders.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 13 October 2010, 15:13 GMT+2
not entirely working version. wpsbuild.pl needs to be fixed so the cabbie theme uses the correct path prefix. I'm not sure the autoconf.h change is correct also.
   filepathchanges.diff (5.9 KiB) 
 b/apps/gui/icon.c                       |    6 +++-
 b/apps/gui/skin_engine/skin_backdrops.c |   16 +++++--------
 b/apps/gui/skin_engine/skin_fonts.c     |    2 -
 b/firmware/common/rbpaths.c             |   39 ++++++++++++++++++++++----------
 b/tools/configure                       |    5 ++++
 5 files changed, 44 insertions(+), 24 deletions(-)

Comment by Thomas Martitz (kugel.) - Wednesday, 13 October 2010, 15:17 GMT+2
What does that patch? I thought the bug this task is about was fixed.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 13 October 2010, 15:22 GMT+2
config.cfg storing real paths which it shouldnt. it should actually store them in the same way regular targets do (i.e wps: /.rockbox/wps/cabbiev2.wps) so you could take a .cfg from your dap and use it on raaa. right now its using /<Rb>/ but the prefix is irrelevant. the important thing is it shouldnt be storing /usr/local/....
Comment by Thomas Martitz (kugel.) - Wednesday, 13 October 2010, 15:25 GMT+2
As I said in my previous post, it doesn't actually use the path from the .cfg. It actually ignores the path (and extension) and only looks at the basefile name in the usual search path order.
Comment by Jonathan Gordon (jdgordon) - Thursday, 14 October 2010, 00:08 GMT+2
that isnt 100% true as I found out. that full path is currently used if the get_user_file_dir() isnt called correctly (like forgetting IS_FILE). and anyway that is more reason to not put the full path in the config.
Comment by Jonathan Gordon (jdgordon) - Thursday, 14 October 2010, 13:14 GMT+2
cleaner version which make ROCKBOX_DIR point back to the same as normal builds, ROCKBOX_SHARE_PATH is the old ROCKBOX_DIR. doing it this way means no scripts need to be fixed to work, and configs can be shared without any problem.

I find it a bit annoying that you really need 2 MAX_PATH buffers in the caller to make it work though.
   filepathchanges.diff (7.7 KiB) 
 b/apps/gui/icon.c                       |    6 +++--
 b/apps/gui/skin_engine/skin_backdrops.c |   15 +++++-------
 b/apps/gui/skin_engine/skin_fonts.c     |    2 -
 b/firmware/common/rbpaths.c             |   38 +++++++++++++++++++++-----------
 b/tools/configure                       |   10 +++++---
 5 files changed, 44 insertions(+), 27 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Monday, 18 October 2010, 14:59 GMT+2
This is more of a proof of concept than committable patch... remove all get_user_file_path() calls in apps/ and transparently change the filename in open() and opendir() to get the right path. sdl app only just now
   filepathchanges.diff (34.6 KiB) 
 b/apps/filetypes.c                      |    8 -
 b/apps/gui/skin_engine/skin_backdrops.c |   15 +--
 b/apps/gui/skin_engine/skin_engine.c    |    4 
 b/apps/gui/skin_engine/skin_fonts.c     |    4 
 b/apps/gui/skin_engine/skin_parser.c    |    3 
 b/apps/main.c                           |    4 
 b/apps/menus/theme_menu.c               |    4 
 b/apps/playlist.c                       |   10 --
 b/apps/playlist_catalog.c               |    8 -
 b/apps/settings.c                       |   36 +------
 b/apps/tagcache.c                       |  145 +++++++++-----------------------
 b/apps/tree.c                           |    8 -
 b/firmware/common/file.c                |    1 
 b/firmware/common/rbpaths.c             |   41 +++++----
 b/firmware/export/rbpaths.h             |   16 +++
 b/tools/configure                       |   10 +-
 b/uisimulator/common/io.c               |   16 +++
 17 files changed, 141 insertions(+), 192 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Sunday, 24 October 2010, 12:55 GMT+2
This changes the android path to /data/data/org.rockbox/.rockbox amd transparently changes file paths called with open() and diropen().
tested with android and sdl.

make reconf doesnt work though and that needs to be fixed before commiting
   filepathchanges.diff (43 KiB) 
 b/android/src/org/rockbox/RockboxService.java |   15 +-
 b/apps/filetypes.c                            |    8 -
 b/apps/gui/icon.c                             |    2 
 b/apps/gui/skin_engine/skin_backdrops.c       |   14 --
 b/apps/gui/skin_engine/skin_engine.c          |    4 
 b/apps/gui/skin_engine/skin_fonts.c           |    4 
 b/apps/gui/skin_engine/skin_parser.c          |    3 
 b/apps/main.c                                 |    4 
 b/apps/menus/theme_menu.c                     |    4 
 b/apps/playlist.c                             |   10 -
 b/apps/playlist_catalog.c                     |    8 -
 b/apps/settings.c                             |   36 +-----
 b/apps/tagcache.c                             |  145 ++++++++------------------
 b/apps/tree.c                                 |    8 -
 b/firmware/common/dircache.c                  |   11 -
 b/firmware/common/rbpaths.c                   |   70 ++++++++----
 b/firmware/export/rbpaths.h                   |   25 ----
 b/firmware/font.c                             |   10 -
 b/firmware/include/file.h                     |    7 +
 b/firmware/load_code.c                        |    6 -
 b/firmware/target/hosted/android/fs-android.c |   11 +
 b/tools/configure                             |   14 +-
 b/uisimulator/common/io.c                     |    5 
 23 files changed, 173 insertions(+), 251 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Sunday, 24 October 2010, 13:34 GMT+2
this version fixes make reconf. please test
   filepathchanges.diff (44.2 KiB) 
 b/android/src/org/rockbox/RockboxService.java |   15 +-
 b/apps/filetypes.c                            |    8 -
 b/apps/gui/icon.c                             |    2 
 b/apps/gui/skin_engine/skin_backdrops.c       |   14 --
 b/apps/gui/skin_engine/skin_engine.c          |    4 
 b/apps/gui/skin_engine/skin_fonts.c           |    4 
 b/apps/gui/skin_engine/skin_parser.c          |    3 
 b/apps/main.c                                 |    4 
 b/apps/menus/theme_menu.c                     |    4 
 b/apps/playlist.c                             |   10 -
 b/apps/playlist_catalog.c                     |    8 -
 b/apps/settings.c                             |   36 +-----
 b/apps/tagcache.c                             |  145 ++++++++------------------
 b/apps/tree.c                                 |    8 -
 b/firmware/common/dircache.c                  |   11 -
 b/firmware/common/file.c                      |    1 
 b/firmware/common/rbpaths.c                   |   70 ++++++++----
 b/firmware/export/rbpaths.h                   |   25 ----
 b/firmware/font.c                             |   10 -
 b/firmware/include/file.h                     |    7 +
 b/firmware/load_code.c                        |    6 -
 b/firmware/target/hosted/android/fs-android.c |   11 +
 b/tools/configure                             |   20 +--
 b/uisimulator/common/io.c                     |    5 
 24 files changed, 174 insertions(+), 257 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Monday, 25 October 2010, 14:38 GMT+2
sync
   filepathchanges.diff (43 KiB) 
 b/android/src/org/rockbox/RockboxService.java |   15 +-
 b/apps/filetypes.c                            |    8 -
 b/apps/gui/icon.c                             |    2 
 b/apps/gui/skin_engine/skin_backdrops.c       |   14 --
 b/apps/gui/skin_engine/skin_engine.c          |    4 
 b/apps/gui/skin_engine/skin_fonts.c           |    4 
 b/apps/gui/skin_engine/skin_parser.c          |    3 
 b/apps/main.c                                 |    4 
 b/apps/menus/theme_menu.c                     |    4 
 b/apps/playlist.c                             |   10 -
 b/apps/playlist_catalog.c                     |    8 -
 b/apps/settings.c                             |   36 +-----
 b/apps/tagcache.c                             |  145 ++++++++------------------
 b/apps/tree.c                                 |    3 
 b/firmware/common/dircache.c                  |   11 -
 b/firmware/common/rbpaths.c                   |   70 ++++++++----
 b/firmware/export/rbpaths.h                   |   25 ----
 b/firmware/font.c                             |   10 -
 b/firmware/include/file.h                     |    7 +
 b/firmware/load_code.c                        |    6 -
 b/firmware/target/hosted/android/fs-android.c |   11 +
 b/tools/configure                             |   20 +--
 b/uisimulator/common/io.c                     |    5 
 23 files changed, 171 insertions(+), 254 deletions(-)

Comment by Thomas Martitz (kugel.) - Sunday, 05 December 2010, 20:36 GMT+2
Sync and extended to wrap all IO functions that take a path.

Works very well, I think I'm going to commit in a day or two.
   app-paths.patch (49.5 KiB) 
 b/android/src/org/rockbox/RockboxService.java |    9 +
 b/apps/filetypes.c                            |    8 -
 b/apps/gui/icon.c                             |    7 -
 b/apps/gui/skin_engine/skin_backdrops.c       |   14 --
 b/apps/gui/skin_engine/skin_engine.c          |    6 
 b/apps/gui/skin_engine/skin_fonts.c           |    4 
 b/apps/gui/skin_engine/skin_parser.c          |    3 
 b/apps/main.c                                 |    4 
 b/apps/menus/theme_menu.c                     |    4 
 b/apps/playlist.c                             |   10 -
 b/apps/playlist_catalog.c                     |    8 -
 b/apps/plugin.c                               |    6 
 b/apps/settings.c                             |   36 +----
 b/apps/tagcache.c                             |  145 +++++++---------------
 b/apps/tree.c                                 |    3 
 b/firmware/common/dircache.c                  |   11 -
 b/firmware/common/rbpaths.c                   |  166 ++++++++++++++++++++++----
 b/firmware/export/rbpaths.h                   |   31 ----
 b/firmware/font.c                             |   10 -
 b/firmware/include/dir.h                      |    7 -
 b/firmware/include/dir_uncached.h             |   33 ++++-
 b/firmware/include/file.h                     |   46 +++++--
 b/firmware/load_code.c                        |    6 
 b/firmware/target/hosted/android/dir-target.h |   21 ++-
 b/firmware/target/hosted/android/fs-android.c |    1 
 b/tools/configure                             |   22 +--
 b/uisimulator/common/io.c                     |    1 
 27 files changed, 324 insertions(+), 298 deletions(-)

Loading...