Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#6203 - unified way to open utf8 text files

Attached to Project: Rockbox
Opened by Dominik Riebeling (bluebrother) - Tuesday, 17 October 2006, 22:57 GMT+2
Last edited by Dominik Riebeling (bluebrother) - Saturday, 02 August 2008, 22:39 GMT+2
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Since Rockbox supports utf8 there has been the issue of text files containing a BOM created by some editors. Rockbox needs to skip that BOM which is currently done independently in various places (and still missing in a couple of places). This patch addresses this by introducing a new open_utf8() function which behaves exactly like open() but places the file pointer to the first text byte after opening. That way opening a text file only needs to use that function and doesn't need to bother with a BOM anymore.

The attached patch changes this in a couple of places (see also commited  FS#5770 , pending  FS#6071 ; I guess I still missed some places where is should get changed). Using lseek() may still break but I'm not sure if that is actually used on text files anywhere. In that case a similar function for seeking could solve this easily.
   open_utf8_2.patch (4.7 KiB) 
 apps/misc.c            |   20 ++++++++++++++++++++
 apps/misc.h            |    1 +
 apps/playlist.c        |   11 +----------
 apps/settings.c        |    2 +-
 apps/gui/gwps-common.c |   15 ---------------
 apps/gui/gwps.c        |    2 +-
 apps/recorder/radio.c  |    2 +-
 7 files changed, 25 insertions(+), 28 deletions(-)

This task depends upon

Closed by  Dominik Riebeling (bluebrother)
Saturday, 02 August 2008, 22:39 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  committed adapted version as r18185.
Comment by Dominik Riebeling (bluebrother) - Sunday, 08 April 2007, 20:37 GMT+2
updated to current svn. Added BOM check to cuesheet.c too (untested, as I don't have any cuesheets around)
   open_utf8_r13076.diff (4.4 KiB) 
 apps/misc.h              |    2 ++
 apps/playlist.c          |   11 +----------
 apps/recorder/radio.c    |    2 +-
 apps/settings.c          |    2 +-
 apps/gui/wps_parser.c    |   18 +-----------------
 apps/misc.c              |   19 +++++++++++++++++++
 apps/recorder/keyboard.c |    2 +-
 apps/cuesheet.c          |    2 +-
 8 files changed, 27 insertions(+), 31 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Thursday, 17 April 2008, 15:35 GMT+2
any reason this hasnt been commited?
Comment by Dominik Riebeling (bluebrother) - Thursday, 17 April 2008, 21:59 GMT+2
I guess it simply wasn't committed because nobody picked it up -- and I somewhat forgot it. As the patch is quite out of date I resynced it (and replaced the BOM check with a somewhat nicer variant I found in playlist.c). It's barely tested right now (wps and playlists seem to work fine, haven't tried other affected functionality yet).

If there are no objections I'd be fine picking it up again and committing it after the missing testing has been done.
   open_utf8_r17152.diff (6.5 KiB) 
 apps/misc.h              |    2 ++
 apps/playlist.c          |   30 +++++-------------------------
 apps/cuesheet.c          |    2 +-
 apps/recorder/radio.c    |    2 +-
 apps/recorder/keyboard.c |    2 +-
 apps/settings.c          |    2 +-
 apps/gui/wps_parser.c    |   18 +-----------------
 apps/misc.c              |   22 ++++++++++++++++++++++
 8 files changed, 34 insertions(+), 46 deletions(-)

Comment by Bertrik Sikken (bertrik) - Friday, 01 August 2008, 19:40 GMT+2
The patch still applies, but the result does not compile anymore. learman added some code to add the BOM to m3u8 files in r17786 that breaks it (requires the BOM #define that was moved to apps/misc.c)
Maybe the utf8_open function could also _write_ a BOM in case it's called with specific flags set (e.g. O_CREAT or O_WRONLY)?
Comment by Dominik Riebeling (bluebrother) - Saturday, 02 August 2008, 00:18 GMT+2
I still have that code around and properly synced. I planned committing it somewhere this weekend (given that I find the time, of course ;-) Making utf8_open to also write the BOM is a nice idea, I'll check it.

Loading...