Refactor mount_data api as to not duplicate mount command functionality. Needs revisiting as mount_data is not even in TIP 430

3 files changed, 61 insertions, 90 deletions

diff --git a/doc/zipfs.3 b/doc/zipfs.3
index af1281c..571647f 100644
--- a/doc/zipfs.3
+++ b/doc/zipfs.3
@@ -111,23 +111,12 @@ is NULL, the path to the archive mounted at that mount point is stored
 as \fIinterp\fR's result. The function returns a standard Tcl result
 code.
 .PP
-\fBTclZipfs_MountBuffer\fR is similar to \fBTclZipfs_Mount\fR except that the
-content of a ZIP archive is passed in the buffer pointed to by \fIdata\fR.
-If \fImountpoint\fR and
-\fIdata\fR are both non-NULL, the function
-mounts the ZIP archive content \fIdata\fR on the mount point
-given in \fImountpoint\fR.
-The
-\fIcopy\fR argument determines whether the buffer is internally copied before
-mounting or not. The ZIP archive is assumed to be not password protected.
-On success, \fIinterp\fR's result is set to the normalized mount point
-path.
-If \fImountpoint\fR is a NULL pointer, information on all currently mounted ZIP
-file systems is stored in \fIinterp\fR's result as a sequence of mount
-points and ZIP file names. If \fImountpoint\fR is not NULL but \fIdata\fR
-is NULL, the path to the archive mounted at that mount point is stored
-as \fIinterp\fR's result. The function returns a standard Tcl result
-code.
+\fBTclZipfs_MountBuffer\fR mounts the ZIP archive content \fIdata\fR on the
+mount point given in \fImountpoint\fR. Both \fImountpoint\fR and \fIdata\fR must
+be specified as non-NULL. The \fIcopy\fR argument determines whether the buffer
+is internally copied before mounting or not. The ZIP archive is assumed to be
+not password protected. On success, \fIinterp\fR's result is set to the
+normalized mount point path.
 .PP
 \fBTclZipfs_Unmount\fR undoes the effect of \fBTclZipfs_Mount\fR, i.e., it
 unmounts the mounted ZIP file system that was mounted from \fIzipname\fR (at
diff --git a/generic/tclZipfs.c b/generic/tclZipfs.c
index d08767b..8ce17eb 100644
--- a/generic/tclZipfs.c
+++ b/generic/tclZipfs.c
@@ -2435,22 +2435,17 @@ TclZipfs_MountBuffer(
     ZipFile *zf;
     int ret;
 
+    if (mountPoint == NULL || data == NULL) {
+	ZIPFS_ERROR(interp, "mount point and/or data are null");
+	return TCL_ERROR;
+    }
+
     /* TODO - how come a *read* lock suffices for initialzing ? */
     ReadLock();
     if (!ZipFS.initialized) {
 	ZipfsSetup();
     }
 
-    /*
-     * No mount point, so list all mount points and what is mounted there.
-     */
-
-    if (!mountPoint) {
-	ret = ListMountPoints(interp);
-	Unlock();
-	return ret;
-    }
-
     Tcl_DString ds;
     Tcl_DStringInit(&ds);
     ret = NormalizeMountPoint(interp, mountPoint, &ds);
@@ -2460,57 +2455,53 @@ TclZipfs_MountBuffer(
     }
     mountPoint = Tcl_DStringValue(&ds);
 
-    if (data == NULL) {
-	/* Mount point but no data, so describe what is mounted at there */
-	ret = DescribeMounted(interp, mountPoint);
-	Unlock();
-    } else {
-	Unlock();
+    Unlock();
 
-	/*
-	 * Have both a mount point and data to mount there.
-	 * What's the magic about 64 * 1024 * 1024 ?
-	 */
-	ret = TCL_ERROR;
-	if ((datalen <= ZIP_CENTRAL_END_LEN) ||
-	    (datalen - ZIP_CENTRAL_END_LEN) >
-		(64 * 1024 * 1024 - ZIP_CENTRAL_END_LEN)) {
-	    ZIPFS_ERROR(interp, "illegal file size");
-	    ZIPFS_ERROR_CODE(interp, "FILE_SIZE");
-	    goto done;
-	}
-	zf = AllocateZipFile(interp, strlen(mountPoint));
-	if (zf == NULL) {
-	    goto done;
-	}
-	zf->isMemBuffer = 1;
-	zf->length = datalen;
+    /*
+     * Have both a mount point and data to mount there.
+     * What's the magic about 64 * 1024 * 1024 ?
+     */
+    ret = TCL_ERROR;
+    if ((datalen <= ZIP_CENTRAL_END_LEN) ||
+	(datalen - ZIP_CENTRAL_END_LEN) >
+	    (64 * 1024 * 1024 - ZIP_CENTRAL_END_LEN)) {
+	ZIPFS_ERROR(interp, "illegal file size");
+	ZIPFS_ERROR_CODE(interp, "FILE_SIZE");
+	goto done;
+    }
+    zf = AllocateZipFile(interp, strlen(mountPoint));
+    if (zf == NULL) {
+	goto done;
+    }
+    zf->isMemBuffer = 1;
+    zf->length = datalen;
 
-	if (copy) {
-	    zf->data = (unsigned char *)attemptckalloc(datalen);
-	    if (zf->data == NULL) {
-		ZipFSCloseArchive(interp, zf);
-		ckfree(zf);
-		ZIPFS_MEM_ERROR(interp);
-		goto done;
-	    }
-	    memcpy(zf->data, data, datalen);
-	    zf->ptrToFree = zf->data;
-	} else {
-	    zf->data = (unsigned char *)data;
-	    zf->ptrToFree = NULL;
-	}
-	ret = ZipFSFindTOC(interp, 1, zf);
-	if (ret != TCL_OK) {
+    if (copy) {
+	zf->data = (unsigned char *)attemptckalloc(datalen);
+	if (zf->data == NULL) {
+	    ZipFSCloseArchive(interp, zf);
 	    ckfree(zf);
-	} else {
-	    /* Note ZipFSCatalogFilesystem will free zf on error */
-	    ret = ZipFSCatalogFilesystem(
-		interp, zf, mountPoint, NULL, "Memory Buffer");
-	}
-	if (ret == TCL_OK && interp) {
-	    Tcl_DStringResult(interp, &ds);
+	    ZIPFS_MEM_ERROR(interp);
+	    goto done;
 	}
+	memcpy(zf->data, data, datalen);
+	zf->ptrToFree = zf->data;
+    }
+    else {
+	zf->data = (unsigned char *)data;
+	zf->ptrToFree = NULL;
+    }
+    ret = ZipFSFindTOC(interp, 1, zf);
+    if (ret != TCL_OK) {
+	ckfree(zf);
+    }
+    else {
+	/* Note ZipFSCatalogFilesystem will free zf on error */
+	ret = ZipFSCatalogFilesystem(
+	    interp, zf, mountPoint, NULL, "Memory Buffer");
+    }
+    if (ret == TCL_OK && interp) {
+	Tcl_DStringResult(interp, &ds);
     }
 
 done:
@@ -2675,21 +2666,14 @@ ZipFSMountBufferObjCmd(
     unsigned char *data = NULL;
     Tcl_Size length;
 
-    if (objc > 3) {
-	Tcl_WrongNumArgs(interp, 1, objv, "?data? ?mountpoint?");
+    if (objc != 3) {
+	Tcl_WrongNumArgs(interp, 1, objv, "data mountpoint");
 	return TCL_ERROR;
     }
-
-    if (objc > 1) {
-	if (objc == 2) {
-	    mountPoint = Tcl_GetString(objv[1]);
-	} else {
-	    data = Tcl_GetBytesFromObj(interp, objv[1], &length);
-	    mountPoint = Tcl_GetString(objv[2]);
-	    if (data == NULL) {
-		return TCL_ERROR;
-	    }
-	}
+    data = Tcl_GetBytesFromObj(interp, objv[1], &length);
+    mountPoint = Tcl_GetString(objv[2]);
+    if (data == NULL) {
+	return TCL_ERROR;
     }
     return TclZipfs_MountBuffer(interp, data, length, mountPoint, 1);
 }
diff --git a/tests/zipfs.test b/tests/zipfs.test
index a7a6a1d..0226918 100644
--- a/tests/zipfs.test
+++ b/tests/zipfs.test
@@ -361,9 +361,6 @@ test zipfs-5.3 {zipfs mount_data: short data} -constraints zipfs -body {
     append data PK\x05\x06.....................................
     zipfs mount_data $data gorp
 } -returnCodes error -result {archive directory truncated}
-test zipfs-5.4 {zipfs mount_data: bad arg count} -constraints zipfs -body {
-    zipfs mount_data {} gorp foobar
-} -returnCodes error -result {wrong # args: should be "zipfs mount_data ?data? ?mountpoint?"}
 
 test zipfs-6.1 {zipfs mkkey} -constraints zipfs -body {
     binary scan [zipfs mkkey gorp] cu* x
@@ -483,6 +480,7 @@ namespace eval test_ns_zipfs {
     }
 
     testnumargs "zipfs mount" "" "?zipfile? ?mountpoint? ?password?"
+    testnumargs "zipfs mount_data" "data mountpoint" ""
 
     # Not supported zip files
     testbadmount non-existent-file    nosuchfile.zip "couldn't open*nosuchfile.zip*no such file or directory"