From a301bc01d00d922c1ce15fa37add27a26d7e17b9 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Thu, 5 Oct 2023 15:39:05 +0000 Subject: [PATCH] Claim all paths under //zipfs:/ as zipfs paths --- doc/zipfs.n | 4 +- generic/tclZipfs.c | 98 ++++------------------------------------------ tests/zipfs.test | 10 +++-- 3 files changed, 16 insertions(+), 96 deletions(-) diff --git a/doc/zipfs.n b/doc/zipfs.n index 26b45a69cd7..7cb050ef526 100644 --- a/doc/zipfs.n +++ b/doc/zipfs.n @@ -141,9 +141,7 @@ on most platforms. \fBzipfs unmount \fImountpoint\fR . Unmounts a previously mounted ZIP archive mounted to \fImountpoint\fR. -If the current directory is located within the mounted archive, -the directory that was previously the current directory is restored -on the unmount. The command will fail with an error exception if +The command will fail with an error exception if there are any files within the mounted archive are open. .SS "ZIP CREATION COMMANDS" This package also provides several commands to aid the creation of ZIP diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c index a50c6927fc2..78c3cb65be0 100644 --- a/generic/tclZipfs.c +++ b/generic/tclZipfs.c @@ -5645,9 +5645,9 @@ ZipFSMatchInDirectoryProc( } if (dirOnly) { /* - * Not found in hash. May be a path that is the ancestor of a mount. - * e.g. glob //zipfs:/a/? with mount at //zipfs:/a/b/c. Also have - * to be careful about duplicates, such as when another mount is + * Also check paths that are ancestors of a mount. e.g. glob + * //zipfs:/a/? with mount at //zipfs:/a/b/c. Also have to be + * careful about duplicates, such as when another mount is * //zipfs:/a/b/d */ Tcl_DString ds; @@ -5796,8 +5796,6 @@ ZipFSPathInFilesystemProc( Tcl_Obj *pathPtr, TCL_UNUSED(void **)) { - Tcl_HashEntry *hPtr; - Tcl_HashSearch search; Tcl_Size len; char *path; @@ -5806,93 +5804,13 @@ ZipFSPathInFilesystemProc( return -1; } path = TclGetStringFromObj(pathPtr, &len); + /* - * TODO - why not make ZIPFS_VOLUME both necessary AND sufficient? - * Currently we only claim ownership if there is a matching mount. + * Claim any path under ZIPFS_VOLUME as ours. This is both a necessary + * and sufficient condition as zipfs mounts at arbitrary paths are + * not permitted (unlike Androwish). */ - if (strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) != 0) { - return -1; - } else if (len == ZIPFS_VOLUME_LEN && ZipFS.zipHash.numEntries != 0) { - /* zipfs root and at least one entry */ - return TCL_OK; - } - - int ret = TCL_OK; - - ReadLock(); - hPtr = Tcl_FindHashEntry(&ZipFS.fileHash, path); - if (hPtr) { - goto endloop; - } - - /* - * Not in hash table but still could be owned by zipfs in two other cases: - * Assuming there is a mount point //zipfs:/a/b/c, - * 1. The path is under the mount point, e.g. //zipfs:/a/b/c/f but that - * file does not exist. - * 2. The path is an intermediate directory in a mount point, e.g. - * //zipfs:/a/b - */ - - for (hPtr = Tcl_FirstHashEntry(&ZipFS.zipHash, &search); hPtr; - hPtr = Tcl_NextHashEntry(&search)) { - ZipFile *zf = (ZipFile *) Tcl_GetHashValue(hPtr); - - if (zf->mountPointLen == 0) { - /* - * Mounted on the root (/) - * TODO - a holdover from androwish? Tcl does not allow mounting - * outside of the //zipfs:/ area. - */ - ZipEntry *z; - - for (z = zf->topEnts; z != NULL; z = z->tnext) { - if (strncmp(path, z->name, len) == 0) { - int lenz = (int)strlen(z->name); - if (len == lenz) { - /* Would have been in hash table? But nm ... */ - goto endloop; - } else if (len > lenz) { - /* Case 1 above */ - if (path[lenz] == '/') { - goto endloop; - } - } else { /* len < lenz */ - /* Case 2 above */ - if (z->name[len] == '/') { - goto endloop; - } - } - } - - } - } else { - /* Not mounted on root - the norm in Tcl core */ - - /* Lengths are known so check them before strnmp for efficiency*/ - assert(len != ZIPFS_VOLUME_LEN); /* Else already handled at top */ - if (len == zf->mountPointLen) { - /* A non-root or root mount. */ - goto endloop; - } else if (len > zf->mountPointLen) { - /* Case 1 above */ - if (path[zf->mountPointLen] == '/' && - strncmp(path, zf->mountPoint, zf->mountPointLen) == 0) { - goto endloop; - } - } else { /* len < zf->mountPointLen */ - if (zf->mountPoint[len] == '/' && - strncmp(path, zf->mountPoint, len) == 0) { - goto endloop; - } - } - } - } - ret = -1; /* Not our file */ - - endloop: - Unlock(); - return ret; + return strncmp(path, ZIPFS_VOLUME, ZIPFS_VOLUME_LEN) ? -1 : TCL_OK; } /* diff --git a/tests/zipfs.test b/tests/zipfs.test index b72e431a2b4..6dbf8347490 100644 --- a/tests/zipfs.test +++ b/tests/zipfs.test @@ -636,13 +636,17 @@ namespace eval test_ns_zipfs { } -result {filesystem is busy} -returnCodes error test zipfs-unmount-3 "Unmount mount with current directory" -setup { + set cwd [pwd] mount [zippath test.zip] } -cleanup { + cd $cwd cleanup } -body { - set cwd [pwd] - cd [file join $defMountPt testdir] - list [pwd] [zipfs unmount $defMountPt] [string equal [pwd] $cwd] + # Current directory does not change on unmount. + # This is the same behavior as when USB pen drive is unmounted + set cwd2 [file join $defMountPt testdir] + cd $cwd2 + list [pwd] [zipfs unmount $defMountPt] [string equal [pwd] $cwd2] } -result [list [file join $defMountPt testdir] {} 1] test zipfs-unmount-nested-1 "unmount parent of nested mount on new directory should not affect nested mount" -setup {