Steve Dower added the comment:
Two posts ago: The caller to getfilesize is only using it to check whether
it's small enough to load the file into memory all at once, so too big is an
okay response (that function is in marshal.c and not used anywhere else).
--
Roundup Robot added the comment:
New changeset b4f9e787b857 by Serhiy Storchaka in branch 'default':
Issue #23152: Move declaration into a header and exclude from stable API.
https://hg.python.org/cpython/rev/b4f9e787b857
--
___
Python tracker
Roundup Robot added the comment:
New changeset 72cf174cc0eb by Serhiy Storchaka in branch 'default':
Issue #23152: Move declarations back to posixmodule.c.
https://hg.python.org/cpython/rev/72cf174cc0eb
--
___
Python tracker rep...@bugs.python.org
STINNER Victor added the comment:
You didn't reply to my question on getfilesize().
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23152
___
___
Changes by Steve Dower steve.do...@microsoft.com:
--
resolution: - fixed
stage: patch review - resolved
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23152
___
Roundup Robot added the comment:
New changeset a824c40e8fc0 by Steve Dower in branch 'default':
Issue #23152: Renames time_t_to_FILE_TIME to _Py_time_t_to_FILE_TIME, removes
unused struct win32_stat and return value
https://hg.python.org/cpython/rev/a824c40e8fc0
--
Roundup Robot added the comment:
New changeset 4f6f4aa0d80f by Steve Dower in branch 'default':
Issue #23152: Implement _Py_fstat() to support files larger than 2 GB on
Windows.
https://hg.python.org/cpython/rev/4f6f4aa0d80f
--
nosy: +python-dev
___
Roundup Robot added the comment:
New changeset 307713759a62 by Steve Dower in branch 'default':
Issue #23152: Renames attribute_data_to_stat to _Py_attribute_data_to_stat
https://hg.python.org/cpython/rev/307713759a62
--
___
Python tracker
STINNER Victor added the comment:
The name of attribute_data_to_stat() and other shared functions must be
prefixed by _Py.
+/* Return size of file in bytes; 0 if unknown or INT_MAX if too big */
static off_t getfilesize(FILE *fp)
Hum since we have a type able yo store the file size and the
Steve Dower added the comment:
The caller to getfilesize is only using it to check whether it's small enough
to load the file into memory all at once, so too big is an okay response
(that function is in marshal.c and not used anywhere else).
The error label just returns back to the
Steve Dower added the comment:
Last call before I let the buildbots be the reviewers :)
--
priority: normal - critical
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23152
___
Steve Dower added the comment:
Anyone interested in reviewing this patch?
--
assignee: - steve.dower
stage: - patch review
type: - crash
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23152
Steve Dower added the comment:
Looks like it was only the _io module needing more updates. New patch attached.
--
Added file: http://bugs.python.org/file37888/py_fstat_3.patch
___
Python tracker rep...@bugs.python.org
Steve Dower added the comment:
Huh, okay, looks like the patch still isn't sufficient (I forgot to put -uall
when testing it...) - there are calls in fileio.c that need changing too.
I'll try and do a thorough sweep of calls to fstat and post another patch in
the next day or so.
--
Changes by Steve Dower steve.do...@microsoft.com:
Added file: http://bugs.python.org/file37845/py_fstat_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23152
___
Steve Dower added the comment:
I updated and tested your patch - basically making posixmodule.c use the new
_Py_stat_struct throughout instead of the Win32 definition. The tests run fine
on my VC14 machine.
Should _Py_fstat be conditioned on Py_LIMITED_API in some way, since it's not
Steve Dower added the comment:
Yeah, that's the sole buildbot currently running VS 2015. I'm expecting to have
more after VS 2015 RC is released, since that will be basically finished.
Until then, I'm also regularly building with the latest internal versions and
tracking issues, but nothing
STINNER Victor added the comment:
Currently test_largefile is failing on the Windows buildbots
Oh yes, I just noticed the following bug on AMD64 Windows8 3.x:
http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x/builds/307/steps/test/logs/stdio
[352/391/1] test_largefile
Assertion
Steve Dower added the comment:
Victor, I've been testing your patch and it's mostly good (a few obscure errors
you'd never find without a compiler), but I think we also need to update the
win32_xstat functions in posixmodule.c, since they all try and use the same
struct.
I don't know how
STINNER Victor added the comment:
Here is a patch adding a new _Py_fstat() function which uses signed 64-bit
integer to store the file size and so is not limited to 2 GB files. I just
moved the code from posixmodule.c to fileutils.c.
The patch replaces calls to fstat() with _Py_stat() (and
STINNER Victor added the comment:
#define fstat _fstati64
This change (alone) is not safe because _fstati64() doesn't use struct stat
but struct __stat64.
I don't like such global define, I may have unexpected side effect. I prefer to
modify directly calls to fstat() in .c files (like the
Steve Dower added the comment:
I prefer your patch too. (I've posted on the other thread about the build
problems, and I'll test this when I get a chance.)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23152
Steve Dower added the comment:
Looks like the easiest fix here is to remove the HAVE_SYS_STAT_H definition and
replace it with the include directly:
/* Define to 1 if you have the sys/stat.h header file. */
/* #define HAVE_SYS_STAT_H 1 */
#ifndef MS_WINCE
/* Rather than define
Steve Dower added the comment:
Okay, I'll try and find a way to redefine it only under Windows. Unfortunately,
the CRT defines fstat() as a function, which makes it hard to redefine without
eagerly including sys/stat.h.
--
___
Python tracker
New submission from Steve Dower:
Currently test_largefile is failing on the Windows buildbots because an fstat()
call in Modules/_io/fileio.c is failing. fstat() returns a 32-bit size, but the
file being opened is larger than 2GB.
This appears to be a change in the CRT where it would
Josh Rosenberg added the comment:
I believe explicitly calling the 64 bit version of a function is usually
frowned upon. At least on *NIX systems, the standard solution is to define
-D_FILE_OFFSET_BITS=64 during the build process, so off_t seamlessly becomes a
64 bit value, and the 64 bit
Antoine Pitrou added the comment:
Josh is right, we would only need to use fstat64() under Windows here. fstat()
under POSIX defines st_size as a off_t, which should usually be large enough
even on modern 32-bit systems.
--
___
Python tracker
Josh Rosenberg added the comment:
Ugh. Looking into it further, POSIX systems tend to support
_FILE_OFFSET_BITS=64, but Windows isn't POSIX-y enough. So Windows might need
to be special cased regardless. Blech.
--
___
Python tracker
28 matches
Mail list logo
