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

reyalp

14120

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

« Reply #60 on: 26 / October / 2015, 00:30:18 »

Advertisements

Quote from: srsa_4c on 25 / October / 2015, 15:07:13

Quote from: reyalp on 24 / October / 2015, 16:54:08
I'll try to do some work on the limits
A suggestion (I don't have a patch ATM):
This check could be extended with a check for name length (not computation efficient, but simple).
We could add a new member to the myDIR_s struct (name length limit), and set its value in CHDKOpenDir(), based on the camera's name and path size limits.

Thanks, I was looking at that code but hadn't thought of using the struct to keep track of the max length.

I made good progress on the test script, but there's still a few things I want to sort out before I ask a bunch of people to run it. As expected, there's quite a bit of weirdness at the limits: crashes, files that can't be deleted on camera etc.

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 #61 on: 29 / October / 2015, 20:59:11 »

Here's a partial patch (the rest depends on the test results).

Code: [Select]

Index: core/chdk-dir.c
===================================================================
--- core/chdk-dir.c	(revision 4277)
+++ core/chdk-dir.c	(working copy)
@@ -48,6 +48,8 @@
     int     islfn;              // long file name is being read (state)
     int     lfnpos;             // position in the long output string
 
+    int     mnl;                // maximum name length
+
     char    fn[FNMAX+1];        // current file name stored here by CHDKReadDir
 } myDIR_s;
 
@@ -94,6 +96,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
@@ -222,12 +226,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;
Index: include/camera.h
===================================================================
--- include/camera.h	(revision 4277)
+++ 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
 //----------------------------------------------------------

Logged

reyalp

14120

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

« Reply #62 on: 29 / October / 2015, 23:44:20 »

edit: Nov 13 to attach latest version of script.

Thanks for the patch.

Here's my current test script.

tl;dr instructions:
Trunk 4277 or later build is required.
Run the script.
Post the logfile A/lfntest.log
If the camera crashes, you can look at the last item tried, and adjust the one of the limits described below to prevent files or paths of that length from being attempted.

I would suggest not running this on an SD card that has anything you wouldn't want to lose on it. I never had to reformat a card, but I did end up with some directories that could only be deleted using a card reader.

Gory details:
The basic approach is to try progressively increasing filename lengths in a series of increasingly deeper directory trees. It doesn't try every possible length, just some ranges I've found or guessed to be significant. It prints the file and path length of each test, just so you can see it's doing something. All the important information in is in the log.

There are many permutations that aren't tried: for example, all the directory names are short, and not much is done to check with total path limits are the same for long and short names. Since it uses lua io, it only checks fopen() behavior, not open(). stat and remove are also checked.

There are two options
Limit total path <=
Limit filename <=

These prevent the script from trying a path (directory + filename) or filename longer than the specified value. This can be used if certain values are found to crash.

All the crashes I've found were based on total path length (that is any path longer than X causes a crash, regardless how much is in the directory or file parts.) Filename limits seem to cause fopen to return an error.

Each file + directory length combination is logged with something like

Code: [Select]

TEST START
check: dir=A pfx=t01 ext=.TXT len=11
long:  A/t01AAAA.TXT
short: A/T01AAAA.TXT
PASS open A/t01AAAA.TXT
PASS stat A/t01AAAA.TXT
PASS stat A/T01AAAA.TXT
PASS idir CHDK_LFN A/ found long
PASS idir CANON_FW A/ found short
PASS read A/t01AAAA.TXT
PASS read A/T01AAAA.TXT
PASS os.remove long
PASS remove long
TEST PASSED

"long:" and "short:" show the filename being tested. The "short" is based on the assumptiont the file will be ~1.EXT. The script checks whether a ~1 file exists before running the test, and logs an error if it does.

open tests opening the long name in write mode.

stat checks that the file was created, using both the long and short names

idir checks directory listing, with and without the chdklfn option (for dryos, vxworks only checks onece). The result of this can be: "found long", "found short", "found long==short" or an error.
Note that the short name can still be returned with chdklfn, if the long name is longer than 99 chars, or otherwise unsupported.
The no-chdklfn option can still return the long name: on vxworks, or in the root directory, or presumably on exfat.

read checks that the file can be read, using both long and short names and checking the content.

os.remove checks that it can delete the file by the long name. If removing by long name fails,
removing by short name is attempted.

The final remove test verifies with stat that the file was removed.

The script logs and flushes to disk after each check, so it should be relatively easy to see what combination triggered a crash.

If the script doesn't crash, a summary like the following is logged at the end:

Code: [Select]

PASSED 57
FAILED 6
path:   2 pass:  18 (  20) fail:  19 (  21)
path:  11 pass:  18 (  29) fail:  19 (  30)
path:  14 pass:  18 (  32) fail:  19 (  33)
path:  15 pass:  18 (  33) fail:  19 (  34)
path:  20 pass:  18 (  38) fail:  19 (  39)
path:  21 pass:  18 (  39) fail:  19 (  40)
path:  48 pass:  11 (  59) fail:-

This shows the maximum file length that passed and minimum length that failed, for each directory depth attempted. In this case (D10), filenames longer than 18 chars generate an error, and the total path length is limited to 59 in options because the camera crashes on paths longer than this.

Limits are discovered are
A540 (vxworks, FAT16)
filename 18 total 255 (>255 crashes)

D10 (DryOS r31, FAT16)
filename 18 total 59 (>59 crashes)

sx160 (DryOS r49, FAT32) and elph130 (Dryos r51, FAT32)
filename 255 total unknown but > 500

The remaining versions of interest are DryOS r39-r47, IIRC some of these cameras had a ~32 char limit. DryOS < r31 might also be different.

VxWorks doesn't matter too much since CHDK has no special LFN support.

I have not tried exfat, but again since CHDK doesn't do anything special, it shouldn't matter too much.

I'm not too concerned about testing every variant before 1.4, if we have a system in place we can add limits for specific cameras as needed.

lfntest.lua (14.08 kB - downloaded 28 times.)

« Last Edit: 13 / November / 2015, 21:06:03 by reyalp »

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 #63 on: 31 / October / 2015, 14:33:27 »

Testing activity isn't overwhelming (no-one else has downloaded the test script to date).

Here's a shared folder, I'll upload my test results there.
I have modified the script slightly (added filename lengths 27..33).
Tested so far (<r47 on FAT16, others on FAT32):
sx100 (r23): name limit 18, path+name limit 255
a3200, ixus115 (r47): name limit 31, path+name limit 59
a3400 (r50), sx280 (r52), ixus150 (r54): name limit 255, path+name limit 499 (script max?)

Logged

reyalp

14120

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

« Reply #64 on: 31 / October / 2015, 15:22:03 »

Quote from: srsa_4c on 31 / October / 2015, 14:33:27

Testing activity isn't overwhelming (no-one else has downloaded the test script to date).Here's a shared folder,

Thanks for testing. If the limits go by dryos rev we just have R20 (s5is only), R39, R43 and R45 left.

Quote

a3400 (r50), sx280 (r52), ixus150 (r54): name limit 255, path+name limit 499 (script max?)

Yes, the script currently doesn't go beyond 243 directory + / + 256 filename (and 256 should always fail in some way, since it's beyond fat specs AFAIK.)

I suspect these cams allow some ridiculous depth, but it would probably be a good idea to go a bit deeper.

edit:
OTOH, a lot of windows software (including explorer) maxes out at 259 so maybe we don't need to worry about stuff longer than that. (https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx#maxpath)

« Last Edit: 31 / October / 2015, 16:03:08 by reyalp »

Logged

Don't forget what the H stands for.

reyalp

14120

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

« Reply #65 on: 31 / October / 2015, 16:32:46 »

Here's an updated script that tests paths 1278, as well as filenames from 29-33
For the longest path, it tests both a very deep tree of 8.3 directories and one with a few long directory names.

All pass on dryos r49, up to 255 filename length.

IMO no need to re-test cameras that have already been tested.

One thing I noticed is that deep directory trees are very slow. I thought the script had hung on the final deep tree, but it finished eventually.

As expected, the same directories are unbrowseable using a card reader under win7.

lfntest.lua (14.08 kB - downloaded 27 times.)

Logged

Don't forget what the H stands for.

reyalp

14120

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

« Reply #66 on: 31 / October / 2015, 18:05:20 »

Tried the patch. I think there's an off by one in the name length limit, but I'm not sure the best place to fix it.

With
#define CAM_MAX_FNAME_LENGTH 18
#define CAM_MAX_PATH_LENGTH 59

on D10, the idir test returns the short name for an 18 char long filename.

Code: [Select]

check: dir=A pfx=T03 ext=.TXT len=13
long:  A/T03AAAAAA.TXT
short: A/T03AAA~1.TXT
...
PASS idir CHDK_LFN A/ found long
PASS idir CANON_FW A/ found short
...
check: dir=A pfx=T04 ext=.TXT len=18
long:  A/T04AAAAAAAAAAA.TXT
short: A/T04AAA~1.TXT
...
PASS idir CHDK_LFN A/ found short
PASS idir CANON_FW A/ found short

The summary still gives the expected pass/fail values, since finding the short name with chdklfn enabled counts as a pass.

It's somewhat hard to test, since the camera isn't able to create names longer than the actual limits, and the limits don't currently prevent other calls from attempting something that would crash. To get around this, I tried sx160 with with the D10 limits defined.

The total limit also seems to be off by 1. The following, total path 59
A/LFN_TEST/1234567.D/1234567.D/1234567.D/123456/t01AAAA.TXT

returns the short name with the d10 limits, and the long name without.

Another way to test the limits were working would be to create the directory tree with a card reader. A script could just do a recursive listing of the tree.

In the future, it might be worth making the wrappers for other functions (open, fopen, stat, remove etc) return an error if the limits would be exceeded.

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 #67 on: 31 / October / 2015, 20:21:33 »

Quote from: reyalp on 31 / October / 2015, 18:05:20

Tried the patch. I think there's an off by one in the name length limit, but I'm not sure the best place to fix it.
...
The total limit also seems to be off by 1.

Thanks for catching, I'll look at that. Forgot to document the way I'm using a certain variable and did not look at the code long enough...

Quote from: reyalp on 31 / October / 2015, 18:05:20

Another way to test the limits were working would be to create the directory tree with a card reader.

I had thought about that, even for doing it with multiple OSes (Win, Linux).

Logged

reyalp

14120

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

« Reply #68 on: 31 / October / 2015, 20:48:07 »

Quote from: srsa_4c on 31 / October / 2015, 20:21:33

I had thought about that, even for doing it with multiple OSes (Win, Linux).

Could just make a zip with an appropriate tree, assuming we don't care about testing stuff longer than the windows 259 limit.

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 #69 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.

Logged