Re: Dropping mmap from the virtual file system



Roland Illig wrote:
I would like to drop the mmap() and munmap() methods from the vfs code.

Leonard den Ottolander wrote:
Why?

Roland Illig wrote:
To make the source code of mc smaller and better understandable. For the virtual file system, it nearly makes no sense to provide an mmap() method, as most of the filesystems would have to implement it using read() anyways.

At least, no vfs other than localfs currently implements the mmap() calls.

Another thing is that mmap() does not work reliably on FreeBSD when using it with files in /proc. The Midnight Commander crashes then with a SIGBUS error.

There would be two methods less that a vfs implementor has to think about.

And here's the patch to do it. Note that mc is still using mmap(), but not in the virtual file system. It does not lose any functionality, just code size.

Roland
Index: src/vfsdummy.h
===================================================================
RCS file: /cvsroot/mc/mc/src/vfsdummy.h,v
retrieving revision 1.5
diff -u -p -r1.5 vfsdummy.h
--- src/vfsdummy.h	3 Dec 2004 19:17:47 -0000	1.5
+++ src/vfsdummy.h	27 Feb 2005 22:16:34 -0000
@@ -36,9 +36,6 @@
 #define mc_chdir chdir
 #define mc_unlink unlink
 
-#define mc_mmap mmap
-#define mc_munmap munmap
-
 static inline int
 return_zero (void)
 {
Index: src/view.c
===================================================================
RCS file: /cvsroot/mc/mc/src/view.c,v
retrieving revision 1.175
diff -u -p -r1.175 view.c
--- src/view.c	8 Feb 2005 09:53:51 -0000	1.175
+++ src/view.c	27 Feb 2005 22:16:34 -0000
@@ -36,9 +36,6 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
-#ifdef HAVE_MMAP
-# include <sys/mman.h>
-#endif
 #include <unistd.h>
 
 #include "global.h"
@@ -64,10 +61,6 @@
 #include "charsets.h"
 #include "selcodepage.h"
 
-#ifndef MAP_FILE
-#define MAP_FILE 0
-#endif
-
 /* Block size for reading files in parts */
 #define READ_BLOCK 8192
 #define VIEW_PAGE_SIZE 8192
@@ -103,11 +96,9 @@ struct WView {
     /* view_update_last_byte() must be called after assignment to datasize */
 
     /* File information */
-    int file;			/* File descriptor (for mmap and munmap) */
+    int file;			/* File with random access */
     FILE *stdfile;		/* Stdio struct for reading file in parts */
     int reading_pipe;		/* Flag: Reading from pipe(use popen/pclose) */
-    int mmapping;		/* Did we use mmap on the file? */
-    size_t mmappedsize;		/* Number of bytes that are mmapped; used only for munmap() */
 
     /* Display information */
     offset_type last;           /* Last byte shown */
@@ -245,12 +236,6 @@ free_file (WView *view)
 {
     size_t i;
 
-#ifdef HAVE_MMAP
-    if (view->mmapping) {
-	mc_munmap (view->data, view->mmappedsize);
-	close_view_file (view);
-    } else
-#endif				/* HAVE_MMAP */
     {
 	if (view->reading_pipe) {
 	    /* Close pipe */
@@ -281,7 +266,7 @@ view_done (WView *view)
     set_monitor (view, off);
 
     /* alex: release core, used to replace mmap */
-    if (!view->mmapping && !view->growing_buffer && view->data != NULL) {
+    if (!view->growing_buffer) {
 	g_free (view->data);
 	view->data = NULL;
     }
@@ -328,6 +313,7 @@ view_growbuf_read_until (WView *view, of
         }
         p = view->block_ptr[view->blocks - 1] + view->growbuf_lastindex;
         bytesfree = VIEW_PAGE_SIZE - view->growbuf_lastindex;
         if (view->stdfile != NULL)
             nread = fread (p, 1, bytesfree, view->stdfile);
         else
@@ -430,13 +415,11 @@ view_handle_editkey (WView *view, int ke
 	    g_new (struct hexedit_change_node, 1);
 
 	if (node) {
-#ifndef HAVE_MMAP
 	    /* alex bcs zaporizhzhe ua: here we are using file copy
 	     * completely loaded into memory, so we can replace bytes in
 	     * view->data array to allow changes to be reflected when
 	     * user switches back to text mode */
 	    view->data[view->edit_cursor] = byte_val;
-#endif				/* !HAVE_MMAP */
 	    node->offset = view->edit_cursor;
 	    node->value = byte_val;
 	    enqueue_change (&view->change_list, node);
@@ -591,26 +574,6 @@ load_view_file (WView *view, int fd, con
 	close_view_file (view);
 	return init_growing_view (view, 0, view->filename);
     }
-#ifdef HAVE_MMAP
-    if ((size_t) st->st_size == st->st_size)
-        view->data = mc_mmap (0, st->st_size, PROT_READ,
-            MAP_FILE | MAP_SHARED, view->file, 0);
-    else
-	view->data = (caddr_t) -1;
-    if ((caddr_t) view->data != (caddr_t) - 1) {
-	/* mmap worked */
-	view->first = 0;
-    view->mmappedsize = st->st_size;
-    view->datasize = st->st_size;
-	view->mmapping = 1;
-    view_update_last_byte (view);
-	return NULL;
-    }
-#endif				/* HAVE_MMAP */
-
-    /* For the OSes that don't provide mmap call, try to load all the
-     * file into memory (alex bcs zaporizhzhe ua).  Also, mmap can fail
-     * for any reason, so we use this as fallback (pavel ucw cz) */
 
     /* Make sure view->s.st_size is not truncated when passed to g_malloc */
     if ((gulong) st->st_size == st->st_size)
@@ -650,7 +613,6 @@ do_view_init (WView *view, const char *_
     view->datasize = 0;
     view->growing_buffer = 0;
     view->reading_pipe = 0;
-    view->mmapping = 0;
     view->blocks = 0;
     view->block_ptr = NULL;
     view->first = 0;
Index: vfs/local.c
===================================================================
RCS file: /cvsroot/mc/mc/vfs/local.c,v
retrieving revision 1.34
diff -u -p -r1.34 local.c
--- vfs/local.c	22 Feb 2005 18:35:23 -0000	1.34
+++ vfs/local.c	27 Feb 2005 22:16:34 -0000
@@ -286,27 +286,6 @@ local_ungetlocalcopy (struct vfs_class *
     return 0;
 }
 
-#ifdef HAVE_MMAP
-caddr_t
-local_mmap (struct vfs_class *me, caddr_t addr, size_t len, int prot, int flags, void *data, off_t offset)
-{
-    int fd = * (int *)data;
-
-    (void) me;
-
-    return mmap (addr, len, prot, flags, fd, offset);
-}
-
-int
-local_munmap (struct vfs_class *me, caddr_t addr, size_t len, void *data)
-{
-    (void) me;
-    (void) data;
-
-    return munmap (addr, len);
-}
-#endif
-
 static int
 local_which (struct vfs_class *me, const char *path)
 {
@@ -348,9 +327,5 @@ init_localfs (void)
     vfs_local_ops.ungetlocalcopy = local_ungetlocalcopy;
     vfs_local_ops.mkdir = local_mkdir;
     vfs_local_ops.rmdir = local_rmdir;
-#ifdef HAVE_MMAP
-    vfs_local_ops.mmap = local_mmap;
-    vfs_local_ops.munmap = local_munmap;
-#endif
     vfs_register_class (&vfs_local_ops);
 }
Index: vfs/sfs.c
===================================================================
RCS file: /cvsroot/mc/mc/vfs/sfs.c,v
retrieving revision 1.69
diff -u -p -r1.69 sfs.c
--- vfs/sfs.c	22 Feb 2005 18:35:23 -0000	1.69
+++ vfs/sfs.c	27 Feb 2005 22:16:35 -0000
@@ -450,9 +450,5 @@ init_sfs (void)
     vfs_sfs_ops.free = sfs_free;
     vfs_sfs_ops.getlocalcopy = sfs_getlocalcopy;
     vfs_sfs_ops.ungetlocalcopy = sfs_ungetlocalcopy;
-#ifdef HAVE_MMAP
-    vfs_sfs_ops.mmap = local_mmap;
-    vfs_sfs_ops.munmap = local_munmap;
-#endif
     vfs_register_class (&vfs_sfs_ops);
 }
Index: vfs/vfs-impl.h
===================================================================
RCS file: /cvsroot/mc/mc/vfs/vfs-impl.h,v
retrieving revision 1.6
diff -u -p -r1.6 vfs-impl.h
--- vfs/vfs-impl.h	2 Sep 2004 13:57:59 -0000	1.6
+++ vfs/vfs-impl.h	27 Feb 2005 22:16:35 -0000
@@ -71,12 +71,6 @@ struct vfs_class {
     int (*ctl) (void *vfs_info, int ctlop, void *arg);
     int (*setctl) (struct vfs_class *me, const char *path, int ctlop,
 		   void *arg);
-#ifdef HAVE_MMAP
-    caddr_t (*mmap) (struct vfs_class *me, caddr_t addr, size_t len,
-		     int prot, int flags, void *vfs_info, off_t offset);
-    int (*munmap) (struct vfs_class *me, caddr_t addr, size_t len,
-		   void *vfs_info);
-#endif
 };
 
 /*
Index: vfs/vfs.c
===================================================================
RCS file: /cvsroot/mc/mc/vfs/vfs.c,v
retrieving revision 1.179
diff -u -p -r1.179 vfs.c
--- vfs/vfs.c	22 Feb 2005 18:35:23 -0000	1.179
+++ vfs/vfs.c	27 Feb 2005 22:16:35 -0000
@@ -742,61 +742,6 @@ vfs_file_class_flags (const char *filena
     return vfs->flags;
 }
 
-#ifdef HAVE_MMAP
-static struct mc_mmapping {
-    caddr_t addr;
-    void *vfs_info;
-    struct vfs_class *vfs;
-    struct mc_mmapping *next;
-} *mc_mmaparray = NULL;
-
-caddr_t
-mc_mmap (caddr_t addr, size_t len, int prot, int flags, int fd, off_t offset)
-{
-    struct vfs_class *vfs;
-    caddr_t result;
-    struct mc_mmapping *mcm;
-
-    if (fd == -1)
-	return (caddr_t) -1;
-    
-    vfs = vfs_op (fd);
-    result = vfs->mmap ? (*vfs->mmap)(vfs, addr, len, prot, flags, vfs_info (fd), offset) : (caddr_t)-1;
-    if (result == (caddr_t)-1){
-	errno = ferrno (vfs);
-	return (caddr_t)-1;
-    }
-    mcm =g_new (struct mc_mmapping, 1);
-    mcm->addr = result;
-    mcm->vfs_info = vfs_info (fd);
-    mcm->vfs = vfs;
-    mcm->next = mc_mmaparray;
-    mc_mmaparray = mcm;
-    return result;
-}
-
-int
-mc_munmap (caddr_t addr, size_t len)
-{
-    struct mc_mmapping *mcm, *mcm2 = NULL;
-    
-    for (mcm = mc_mmaparray; mcm != NULL; mcm2 = mcm, mcm = mcm->next){
-        if (mcm->addr == addr){
-            if (mcm2 == NULL)
-            	mc_mmaparray = mcm->next;
-            else
-            	mcm2->next = mcm->next;
-	    if (mcm->vfs->munmap)
-	        (*mcm->vfs->munmap)(mcm->vfs, addr, len, mcm->vfs_info);
-            g_free (mcm);
-            return 0;
-        }
-    }
-    return -1;
-}
-
-#endif
-
 static char *
 mc_def_getlocalcopy (const char *filename)
 {
Index: vfs/vfs.h
===================================================================
RCS file: /cvsroot/mc/mc/vfs/vfs.h,v
retrieving revision 1.122
diff -u -p -r1.122 vfs.h
--- vfs/vfs.h	18 Feb 2005 21:15:37 -0000	1.122
+++ vfs/vfs.h	27 Feb 2005 22:16:35 -0000
@@ -1,10 +1,6 @@
 #ifndef MC_VFS_VFS_H
 #define MC_VFS_VFS_H
 
-#ifdef HAVE_MMAP
-#include <sys/mman.h>
-#endif
-
 void vfs_init (void);
 void vfs_shut (void);
 
@@ -48,10 +44,6 @@ char *mc_getlocalcopy (const char *pathn
 int mc_ungetlocalcopy (const char *pathname, const char *local, int has_changed);
 int mc_ctl (int fd, int ctlop, void *arg);
 int mc_setctl (const char *path, int ctlop, void *arg);
-#ifdef HAVE_MMAP
-caddr_t mc_mmap (caddr_t, size_t, int, int, int, off_t);
-int mc_munmap (caddr_t addr, size_t len);
-#endif				/* HAVE_MMAP */
 
 /* Operations for mc_ctl - on open file */
 enum {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]