Luke wrote:
- why am i the only person filling out the bugreports? :)
You got lucky and found a good test suite! If you put up a nice easy recipe, you'll probably get some help.
- surely these regression tests would work equally as well under the
python2.5.2.exe that you can get as part of the pre-prepared python win32 install that comes from python.org and activestate.com, you don't _need_ my wine-built version of python.exe
Indeed. (For the curious, see http://docs.python.org/library/test.html )
- why am i considering rewriting the tests as c-code when they can
just as well be left as python code?
You only have to do that for the tests that fail. It's useful because a) it greatly reduces the size of the log a wine developer has to look at, and b) adding it to wine's test suite makes it much less likely to recur.
I agree we should be running the Python tests routinely, but probably not as part of wine's "make test". - Dan
Luke wrote:
[MSVCRT__filbuf() needs to strip CR's.]
Here's a quickie patch that kind of gets there, but fails an ftell test. My in-laws arrive in ten minutes, so I probably can't go any further on it this weekend.
There are probably other simplifications that could be made beyond this, I didn't rip out all the scattered places that were involved in \r removal. - Dan
give it a bit more time - there's another one (involving fgets followed by fread) which i'm investigating, i'll have that one nailed as a test case in a bit, am just about to add a test case to #16790
On Sat, Jan 17, 2009 at 7:15 PM, Dan Kegel dank@kegel.com wrote:
Luke wrote:
[MSVCRT__filbuf() needs to strip CR's.]
Here's a quickie patch that kind of gets there, but fails an ftell test. My in-laws arrive in ten minutes, so I probably can't go any further on it this weekend.
There are probably other simplifications that could be made beyond this, I didn't rip out all the scattered places that were involved in \r removal.
- Dan
spiffo, dan, jolly good show :)
that fixed both #16982 _and_ #16970 - the character reading bit - with the exception that the ftell() position, which was wrong _before_ this patch, is still also wrong, as you suspected.
l.
On Sat, Jan 17, 2009 at 7:15 PM, Dan Kegel dank@kegel.com wrote:
Luke wrote:
[MSVCRT__filbuf() needs to strip CR's.]
Here's a quickie patch that kind of gets there, but fails an ftell test. My in-laws arrive in ten minutes, so I probably can't go any further on it this weekend.
There are probably other simplifications that could be made beyond this, I didn't rip out all the scattered places that were involved in \r removal.
- Dan
lkcl@gonzalez:~/src/python2.5-2.5.2$ ./python.exe Lib/test/test_file.py -v testIteration (__main__.OtherFileTests) ... ok
---------------------------------------------------------------------- Ran 1 test in 0.008s
OK
yaay!
so, obviously, the regression tests in python aren't _that_ good, as they haven't spotted the seek position off-by-one bug.
On Sat, Jan 17, 2009 at 8:40 PM, Luke Kenneth Casson Leighton lkcl@lkcl.net wrote:
spiffo, dan, jolly good show :)
that fixed both #16982 _and_ #16970 - the character reading bit - with the exception that the ftell() position, which was wrong _before_ this patch, is still also wrong, as you suspected.
l.
On Sat, Jan 17, 2009 at 7:15 PM, Dan Kegel dank@kegel.com wrote:
Luke wrote:
[MSVCRT__filbuf() needs to strip CR's.]
Here's a quickie patch that kind of gets there, but fails an ftell test. My in-laws arrive in ten minutes, so I probably can't go any further on it this weekend.
There are probably other simplifications that could be made beyond this, I didn't rip out all the scattered places that were involved in \r removal.
- Dan
Luke Kenneth Casson Leighton wrote:
l.
On Sat, Jan 17, 2009 at 7:15 PM, Dan Kegel dank@kegel.com wrote:
Luke wrote:
[MSVCRT__filbuf() needs to strip CR's.]
Here's a quickie patch that kind of gets there, but fails an ftell test. My in-laws arrive in ten minutes, so I probably can't go any further on it this weekend.
There are probably other simplifications that could be made beyond this, I didn't rip out all the scattered places that were involved in \r removal.
- Dan
spiffo, dan, jolly good show :)
that fixed both #16982 _and_ #16970 - the character reading bit - with the exception that the ftell() position, which was wrong _before_ this patch, is still also wrong, as you suspected.
If you can find the location of this problem in the code and fix it, can you please post a patch here first for comment?
It is great that you found the bug, and you might find that not too many folks run Python on Wine as it is much easier to do this natively, both for Linux and the Mac.
James McKenzie
Sadly, I'm having trouble seeing how to do it this way and get ftell() right. ftell returns the position of the fd minus the number of bytes in the buffer. For that to work with this scheme, I'd have to store the number of CRs removed... and I'm not sure where in struct iobuf that could go, or whether native does it that way. (Maybe the high byte of intbuf?)
The problem is illustrated by
#include <stdio.h> #include <string.h>
int main(int argc, char *argv[]) { int i; char buffer[512];
FILE *f = fopen("tst", "w"); fwrite("a\n", 1, 2, f); fwrite("bcd\n", 1, 4, f); fclose(f);
f = fopen("tst", "r"); memset(buffer, 0, 512); fgets(buffer, 512, f); printf("line 1: fgets read %d bytes, '%s'\n", strlen(buffer), buffer); if (memcmp(buffer, "a\n", 3)) printf("Fail! first line wrong, expected 'a\n'\n"); printf("filepos: %ld\n", ftell(f));
if (ftell(f) != 3) printf("Fail! ftell wrong, expected 3\n"); fclose(f); }
which outputs filepos 4 instead of 3 with my patch, and fails with "ftell wrong". - Dan
On Sat, Jan 17, 2009 at 12:40 PM, Luke Kenneth Casson Leighton lkcl@lkcl.net wrote:
spiffo, dan, jolly good show :)
that fixed both #16982 _and_ #16970 - the character reading bit - with the exception that the ftell() position, which was wrong _before_ this patch, is still also wrong, as you suspected.
l.
On Sat, Jan 17, 2009 at 7:15 PM, Dan Kegel dank@kegel.com wrote:
Luke wrote:
[MSVCRT__filbuf() needs to strip CR's.]
Here's a quickie patch that kind of gets there, but fails an ftell test. My in-laws arrive in ten minutes, so I probably can't go any further on it this weekend.
There are probably other simplifications that could be made beyond this, I didn't rip out all the scattered places that were involved in \r removal.
- Dan
-
Dan Kegel -
James McKenzie -
Luke Kenneth Casson Leighton