Long filenames - a file browser modification for Select Script File menu - page 8 - General Discussion and Assistance

reyalp

14118

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #70 on: 02 / November / 2015, 01:27:29 »

Advertisements

Quote from: srsa_4c on 31 / October / 2015, 20:57:15

Quote from: reyalp on 31 / October / 2015, 20:48:07
Could just make a zip with an appropriate tree,
Good idea, extracting is easier than running a script on an unknown system.
Quote
assuming we don't care about testing stuff longer than the windows 259 limit.
That length is big enough to cover all potential uses.

Here's an initial attempt, and a script to try to list and stat the files. Unzip the zip onto the root of the card with a card reader, and then run the script.

fntest2.log records the longest filename and total path length returned by listdir, as well as which could be stat'd.

It looks like the 99 char CHDK limit is also off by 1, as a 98 char filename returns the short name.

It turns out that the 59 char limit on R31 only applies to some operations. Using chdkptp

Code: [Select]

=return os.stat('A/TESTFN/12345678/12345678/12345678/12345678/12/T018AAAAAAAAAA.TXT')

works fine, as does

Code: [Select]

=fh=io.open('A/TESTFN/12345678/12345678/12345678/12345678/12/T018AAAAAAAAAA.TXT','rb') s=fh:read() fh:close() return s

On the other hand,

Code: [Select]

=fh=io.open('A/TESTFN/12345678/12345678/12345678/12345678/12/W018AAAAAAAAAA.TXT','wb') fh:write('foo') fh:close()

crashes on the close. On rebooting, the file exists and contains 'foo'

Code: [Select]

=os.remove('A/TESTFN/12345678/12345678/12345678/12345678/12/W018AAAAAAAAAA.TXT')

Crashes and does not remove the file.

Both crashes are exception 0x0C in FsIoNotifyTa0. I haven't looked into the details.

TESTFN.zip (23.03 kB - downloaded 32 times.)

lfntest2.lua (2.22 kB - downloaded 28 times.)

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #71 on: 04 / November / 2015, 13:28:56 »

After some debugging, I think I found the source of those off-by-one errors. I forgot that LFN entries mostly include a terminating zero char (depending on name length).

Code: [Select]

Index: core/chdk-dir.c
===================================================================
--- core/chdk-dir.c	(revision 4278)
+++ core/chdk-dir.c	(working copy)
@@ -21,8 +21,9 @@
 /*
  * Only a restricted set of ascii characters is supported in long filenames, falls back to short names otherwise
  * Uses less memory than the cameras' ReadFDir functions
- * Only meant to be used on earlier DryOS cameras which don't support exFAT
  * v 1.1: removed CHDK wrappers
+ * v 1.2: - add basic filename and total path length limit check (return short names when they are exceeded)
+ *        - bugfix: ignore terminating zero char of LFN
  */
 
 #include "stdlib.h"
@@ -46,8 +47,10 @@
         unsigned char *feu;
     };
     int     islfn;              // long file name is being read (state)
-    int     lfnpos;             // position in the long output string
+    int     lfnpos;             // position in the long output string (starts from 1 due to the terminating zero char)
 
+    int     mnl;                // maximum name length
+
     char    fn[FNMAX+1];        // current file name stored here by CHDKReadDir
 } myDIR_s;
 
@@ -94,6 +97,8 @@
                 dir->fn[0] = 0;
                 dir->dc = dirc;
                 dir->cp = -1;
+                // determine name length limit ('-1' is due to the extra '/' in a full filename)
+                dir->mnl = MIN(CAM_MAX_FNAME_LENGTH,CAM_MAX_PATH_LENGTH-strlen(name)-1);
                 return dir;
             }
             else
@@ -148,7 +153,7 @@
     {
         // read unicode char, reading as halfword is not working on armv5 due to alignment
         int uch = *(unsigned char*)(dir->fe+lfnchpos[n])+((*(unsigned char*)(dir->fe+lfnchpos[n]+1))<<8);
-        if (uch != 0xffff)
+        if ((uch != 0xffff) && (uch != 0))  // unused space is filled with '0xffff' chars and zero or one '0x0' char
         {
             if ( (check_fn_char(uch) < 0) ) // disable lfn if any chars are outside 7bit ascii
             {
@@ -222,12 +227,13 @@
                     cs = (((cs & 1) << 7) | ((cs & 0xfe) >> 1)) + dir->feu[n];
                 }
                 // checksum computed
-                if ( cs == lfnchsum ) // lfn is valid and belongs to this short name -> return
+                if ( (cs == lfnchsum) && (dir->lfnpos-1 <= dir->mnl) ) 
                 {
+                    // lfn is valid, not too long, and belongs to this short name -> return
                     strcpy(dd, (dir->fn)+FNMAX-dir->lfnpos+1);
                     return (int)((dir->fn)+FNMAX-dir->lfnpos+1);
                 }
-                else // invalid checksum, try re-interpreting entry
+                else // invalid checksum or name too long, try re-interpreting entry
                 {
                     rewind_entry(dir);
                     dir->islfn = 0;
@@ -249,7 +255,7 @@
                     }
                     dir->islfn = dir->fe[0]; // number of lfn entries left + 1
                     read_lfn_entry(dir);
-                    if ( dir->lfnpos > 99 ) // CHDK limit (100 chars) hit, skip lfn
+                    if ( dir->lfnpos > 99+1 ) // CHDK limit (100 chars) hit, skip lfn
                     {
                         dir->islfn = 0;
                         continue;
Index: include/camera.h
===================================================================
--- include/camera.h	(revision 4278)
+++ include/camera.h	(working copy)
@@ -268,6 +268,10 @@
     #define KBD_REPEAT_DELAY                175
     #define KBD_INITIAL_DELAY               500
 
+    // Below limits are for the CHDK LFN support routines (DryOS only)
+    #define CAM_MAX_FNAME_LENGTH            99  // camera can safely use filenames not longer than this (99 is a CHDK limit actually)
+    #define CAM_MAX_PATH_LENGTH             255 // maximum path+filename length
+
 //----------------------------------------------------------
 // Overridden values for each camera
 //----------------------------------------------------------

As noted in the patch, this approach is very simplistic: returned (short) names can still cause problems if the total path+name length is exceeded.

The pre-created files/dirs were really helpful during tests with CHDKPTP.

Logged

Srandista

6
IXUS80 1.01A, IXUS140 1.00B

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #72 on: 06 / November / 2015, 04:03:20 »

Hopefully I can also join this discussion, even if I'm not developer.

I did testing on my two IXUS cameras, IXUS80 with FAT16 and IXUS140 with FAT32. You can find logs attached in this post. Both test done on latest 1.4 trunk (b4278).

LFNTEST2-IXUS80.LOG (6.03 kB - downloaded 25 times.)

LFNTEST2-IXUS140.LOG (6 kB - downloaded 21 times.)

« Last Edit: 06 / November / 2015, 04:11:37 by Srandista »

Logged

reyalp

14118

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #73 on: 07 / November / 2015, 01:34:59 »

Quote from: srsa_4c on 04 / November / 2015, 13:28:56

After some debugging, I think I found the source of those off-by-one errors. I forgot that LFN entries mostly include a terminating zero char (depending on name length).

Thanks, this seems to work right in my tests, both the lfntest2 tree and lfntest with d10 limits on a >=r49 cam. I'll check this in over the weekend if no one objects.

On improvement I'd like to make is set the default limits based on dryos version (after the platform_camera.h include, if not defined), but we currently only have defines for a few versions. PLATFORMOSVER is defined for most (if not all?) dryos cams, but is not currently passed to gcc. I've wanted to replace the CAM_DRYOS_2_3... defines with PLATFORMOSVER, but don't want to get into that for 1.4.

Quote

As noted in the patch, this approach is very simplistic: returned (short) names can still cause problems if the total path+name length is exceeded.

Yes, it's not great, but the same situation exists with the stock firmware. My main goal is to minimize the chance of breaking stuff people are using successfully in 1.3.

We could use the limits to return errors from some of the wrappers instead of crashing, but I don't think that has to be done for 1.4. The fact that different functions have different limits makes this more complicated.

I also thought about making functions convert their arguments to a short name, but this wouldn't work for open creating a file, and could be slow. It also wouldn't solve the problem where an all short name path is too long.

Logged

Don't forget what the H stands for.

reyalp

14118

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #74 on: 07 / November / 2015, 01:51:16 »

Quote from: Srandista on 06 / November / 2015, 04:03:20

Hopefully I can also join this discussion, even if I'm not developer.

I did testing on my two IXUS cameras, IXUS80 with FAT16 and IXUS140 with FAT32. You can find logs attached in this post. Both test done on latest 1.4 trunk (b4278).

Thanks, those look as expected.

If you can try the script from http://chdk.setepontos.com/index.php?topic=10833.msg125221#msg125221 on ixus80 that would be helpful.

It might crash on the default settings. If it does, you can post whatever appears in the log here, or try setting "limit total path" to 255.

No need to test the ixus140 since I have that cam.

Logged

Don't forget what the H stands for.

Srandista

6
IXUS80 1.01A, IXUS140 1.00B

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #75 on: 07 / November / 2015, 04:12:54 »

I did tests with previous script you mention.

On default settings, script was terminated, with limited total path, it successfully finished. You can see results in attached logs.

LFNTEST.LOG (148.74 kB - downloaded 20 times.)

LFNTEST255.LOG (86.41 kB - downloaded 24 times.)

Logged

reyalp

14118

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #76 on: 07 / November / 2015, 18:51:51 »

I've checked in the patch from srsa in post 71. This will affect test results, but shouldn't prevent determining the limits with lfntest.lua.

We still need results from the r39-r47 range.

Quote from: Srandista on 07 / November / 2015, 04:12:54

I did tests with previous script you mention.

On default settings, script was terminated, with limited total path, it successfully finished. You can see results in attached logs.

Thanks, looks like it behaves the same as sx100, the other DryOS R23 cam tested.

Logged

Don't forget what the H stands for.

msl

1280
A720 IS, SX220 HS 1.01a

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #77 on: 08 / November / 2015, 04:21:24 »

Quote from: reyalp on 07 / November / 2015, 18:51:51

We still need results from the r39-r47 range.

Attached is the result of the SX220 (DRYOS R47).

msl

LFNTEST2.LOG (6.17 kB - downloaded 22 times.)

Logged

CHDK-DE: CHDK-DE links

reyalp

14118

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #78 on: 08 / November / 2015, 15:07:29 »

Quote from: msl on 08 / November / 2015, 04:21:24

Quote from: reyalp on 07 / November / 2015, 18:51:51
We still need results from the r39-r47 range.
Attached is the result of the SX220 (DRYOS R47).

msl

Thanks, can you also check with the script in: http://chdk.setepontos.com/index.php?topic=10833.msg125221#msg125221

Logged

Don't forget what the H stands for.

msl

1280
A720 IS, SX220 HS 1.01a

Re: Long filenames - a file browser modification for Select Script File menu

« Reply #79 on: 08 / November / 2015, 15:38:24 »

No problem. Attached is the log file. The camera crashes when len=99.

msl

LFNTEST.LOG (4.07 kB - downloaded 20 times.)

Logged

CHDK-DE: CHDK-DE links