Using lseek inside map_file is unfortunate, better use fstat #5543
Comments
|
Comment author: @xavierleroy 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. |
|
Comment author: gerd Ok, I'll also replace the other lseek/write with an ftruncate, at least if lseek is not possible. Working on a patch. |
|
Comment author: gerd 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. |
|
Comment author: @xavierleroy 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. |
|
Comment author: gerd The results so far are good:
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. |
|
Comment author: gerd Uploaded new patch mmap_growfile implementing the suggested changes. |
|
Comment author: @xavierleroy Second patch merged in 4.00 branch (r12316) and in trunk (r12317). |
Original bug ID: 5543
Reporter: gerd
Status: closed (set by @xavierleroy on 2013-08-31T10:48:57Z)
Resolution: fixed
Priority: normal
Severity: tweak
Version: 3.12.1
Category: otherlibs
Monitored by: mehdi
Bug description
The 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 information
Suggested 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>
File attachments
The text was updated successfully, but these errors were encountered: