Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005543OCamlOCaml otherlibspublic2012-03-16 02:372013-08-31 12:48
Reportergerd 
Assigned To 
PrioritynormalSeveritytweakReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.12.1 
Target VersionFixed in Version 
Summary0005543: Using lseek inside map_file is unfortunate, better use fstat
DescriptionThe problem is that there are file descriptors that do not support seeking, but that are mappable. For example, this is the case for the descriptors returned by shm_create(). lseek is not listed in the allowed operations, but fstat is. Of course, a shared memory object has a size, and it is normally perfectly usable with map_file.

The problem occurs on BSD systems, just saw it on OS X (but I remember I saw it already on FreeBSD), which returns ESPIPE.
Additional InformationSuggested fix: In otherlibs/bigarray/mmap_unix.c, replace the lines

currpos = lseek(fd, 0, SEEK_CUR);
if (currpos == -1) {
  ...
}

by

if (fstat(fd, &st) == -1) {
  ...
}
currpos = st.st_size;

Also declare:

struct stat st;

and

#include <sys/stat.h>
TagsNo tags attached.
Attached Filesdiff file icon mmap.diff [^] (4,110 bytes) 2012-03-25 15:57 [Show Content]
diff file icon mmap_growfile.diff [^] (4,000 bytes) 2012-04-02 13:42 [Show Content]

- Relationships

-  Notes
(0007091)
xleroy (administrator)
2012-03-16 10:10

Yes, that's a good suggestion. lseek() hackery would still be needed in the case where the file needs to be grown before mmap()-ing, but not in the other case. I would gladly consider a patch, esp. if it's been tested.
(0007096)
gerd (reporter)
2012-03-16 22:23
edited on: 2012-03-16 22:25

Ok, I'll also replace the other lseek/write with an ftruncate, at least if lseek is not possible. Working on a patch.

(0007152)
gerd (reporter)
2012-03-25 16:00

So, produced a patch which hopefully catches all cases. Comments are inside the patch. On a modern OS lseek is never used, but the lseek code is still available for older systems.

Testing so far only on 64 bit Linux. Will try to do some more testing the next days.
(0007154)
xleroy (administrator)
2012-03-26 10:30

Thanks for the highly nontrivial patch :-) A cosmetic suggestion: for clarity, maybe the machinery to grow the file should be moved to a separate static "grow_file" function. Let us know how the testing goes.
(0007261)
gerd (reporter)
2012-04-02 12:44

The results so far are good:

 - FreeBSD 7.2 (32 bit)
 - Opensolaris 8.11 (32 bit)
 - Mac OSX 10.5 (32 bit)
 - Linux 2.6.26/glibc 2.11.3 (64 bit)

On the latter system I especially tested mmapping a large file (8GB).

I did not find any system lacking pwrite support. The code using lseek as fallback is a bit questionable because of this - but maybe we should keep it for supporting exotic systems.

I'll see whether I can also do a test on Cygwin.
(0007262)
gerd (reporter)
2012-04-02 13:43

Uploaded new patch mmap_growfile implementing the suggested changes.
(0007275)
xleroy (administrator)
2012-04-04 14:12

Second patch merged in 4.00 branch (r12316) and in trunk (r12317).

- Issue History
Date Modified Username Field Change
2012-03-16 02:37 gerd New Issue
2012-03-16 10:10 xleroy Note Added: 0007091
2012-03-16 10:10 xleroy Status new => acknowledged
2012-03-16 22:23 gerd Note Added: 0007096
2012-03-16 22:25 gerd Note Edited: 0007096 View Revisions
2012-03-25 15:57 gerd File Added: mmap.diff
2012-03-25 16:00 gerd Note Added: 0007152
2012-03-26 10:30 xleroy Note Added: 0007154
2012-03-26 10:30 xleroy Status acknowledged => feedback
2012-04-02 12:44 gerd Note Added: 0007261
2012-04-02 12:44 gerd Status feedback => new
2012-04-02 13:42 gerd File Added: mmap_growfile.diff
2012-04-02 13:43 gerd Note Added: 0007262
2012-04-04 14:12 xleroy Note Added: 0007275
2012-04-04 14:12 xleroy Status new => resolved
2012-04-04 14:12 xleroy Resolution open => fixed
2013-08-31 12:48 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker