Patches item #1706039, was opened at 2007-04-23 17:21
Message generated for change (Comment added) made by josm
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1706039&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: jos (josm)
Assigned to: Nobody/Anonymous (nobody)
Summary: Added clearerr() to clear EOF state
Initial Comment:
This patch is to fix bug 1523853.
Now file.read() works as written in the doc, which says
"An empty string is returned when EOF is encountered immediately."
Before this fix, An empty string was returned even when the last
read() encountered EOF.
----------------------------------------------------------------------
>Comment By: jos (josm)
Date: 2007-05-03 10:03
Message:
Logged In: YES
user_id=1776568
Originator: YES
The flag checking is revised.
I removed feof() so the checking logic become shorter.
I didn't factor out that logic
because I think the logic is simple enough
Added a comment that mentions bug 1523853.
File Added: eof2.diff
----------------------------------------------------------------------
Comment By: Alexander Belopolsky (belopolsky)
Date: 2007-05-01 14:38
Message:
Logged In: YES
user_id=835142
Originator: NO
The latest patch works on Mac OS 10.4 . I would suggest a few cosmetic
changes:
1. Put the flag checking logic in a separate function and call it instead
of fread. This will eliminate code duplication.
2. "else" is redundant in
+ if (ferror(stream)) {
+ clearerr(stream);
+ PyErr_SetFromErrno(PyExc_IOError);
+ return 0;
+ } else if (feof(stream)) {
+ clearerr(stream);
+ }
The flow will be more obvious if you write
+ if (ferror(stream)) {
+ clearerr(stream);
+ PyErr_SetFromErrno(PyExc_IOError);
+ return 0;
+ }
+ if (feof(stream)) {
+ clearerr(stream);
+ }
3. Please mention bug 1523853 in a comment near the test.
otherwise, the patch looks fine.
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-04-28 17:03
Message:
Logged In: YES
user_id=1776568
Originator: YES
New patch Attached.
I believe this one includes all belopolsky's suggestions.
Now all dirty jobs are done in Py_UniversalNewlineFread:
1. calls clearerr if ferror or feof
2. set IOErrorr exception and return
The caller just checks whether IOError is occurred or not
and does appropriate jobs.
The attachment also contains unittest for this problem.
File Added: eof.diff
----------------------------------------------------------------------
Comment By: Alexander Belopolsky (belopolsky)
Date: 2007-04-25 03:14
Message:
Logged In: YES
user_id=835142
Originator: NO
I have just verified that this issue affects readlines and readinto
methods on Mac OS X as well. This tells me that rather than patching all
the individual methods, maybe Py_UniversalNewlineFread should be fixed
instead. I believe that the current situation where a function that looks
like Python API reports errors by setting flags on a FILE object instead of
settin the an exception is against python's design. I suggest changing
Py_UniversalNewlineFgets so that if fread returns 0, it checks whether that
was due to EOF or an error and sets an exception in a case of an error and
always calls clearerr.
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-04-24 17:15
Message:
Logged In: YES
user_id=1776568
Originator: YES
I confirmed that fread can read data from fp in EOF state on Linux. On OS
X, freading from fp in EOF state doesn't work,
just return 0.
I tested this behavior using the following code.
----------------------------
#include <stdio.h>
#include <stdlib.h>
main(void)
{
FILE *fp1, *fp2;
int n;
char buf[128];
fp1 = fopen("spam.txt", "w+");
fputs("foo", fp1);
fseek(fp1, 0L, SEEK_END);
if (fread(buf, 1, sizeof(buf), fp1) == 0)
fprintf(stderr, "first fread failed\n");
fp2 = fopen("spam.txt", "a+");
fputs("bar", fp2);
fclose(fp2);
if (feof(fp1))
fprintf(stderr, "after appending some text, fp1 is still eof\n");
if (fread(buf, 1, sizeof(buf), fp1) == 0)
fprintf(stderr, "second fread failed\n");
printf("buf: %s\n", buf);
fclose(fp1);
return 0;
}
----------------------------
=============
On Linux
=============
first fread failed
after appending some text, fp1 is still eof
buf: bar
=============
On OS X
=============
first fread failed
after appending some text, fp1 is still eof
second fread failed
buf:
Anyway, I think it's safe and preferable to clear EOF indicator in this
case.
----------------------------------------------------------------------
Comment By: Alexander Belopolsky (belopolsky)
Date: 2007-04-24 04:28
Message:
Logged In: YES
user_id=835142
Originator: NO
Hmm. It looks like glibc's fread will attempt reading even if EOF flag is
already set.
I've reproduced the problem on OS X 10.4.9 and Python 2.6a0
(trunk:54926M).
This patch fixes it.
----------------------------------------------------------------------
Comment By: jos (josm)
Date: 2007-04-23 23:27
Message:
Logged In: YES
user_id=1776568
Originator: YES
I can easily reproduce this problem on OS X 10.4.9 using Python 2.4.3 and
Python 2.5.
Here's the code
#################
import os, sys
filename = "spam"
f = open(filename, "w+")
f.seek(0, 2)
line = f.read() # EOF
# Writing the same file using another fd
f2 = open(filename, "a+")
f2.write("Spam")
f2.flush()
f2.close()
statinfo = os.stat(filename)
print "file size: %d" % statinfo.st_size
print "position : %d" % f.tell()
line = f.read() # read() returns emtpy!! readlines?() works ok
print "line : [%s]" % line
#################
On my machine, it outputs the following
###
file size: 4
position : 0
line : []
###
And just now I found that on ubuntu on the same machine (vmware),
it works collect.
###
file size: 4
position : 0
line : [Spam]
###
My patched version python outputs the same result as above on OS X.
We need clearerr() there, too because clearerr()'s job is not only only
clearing error indicators
but EOF indicators also.
----------------------------------------------------------------------
Comment By: Alexander Belopolsky (belopolsky)
Date: 2007-04-23 20:22
Message:
Logged In: YES
user_id=835142
Originator: NO
I was not able to reproduce the problem. I tried to read through the end
of the file, append to the file from an external process and read again.
It worked as expected: more data was read.
The proposed patch seems to be redundant: once ferror(f->f_fp) is known to
be 0, clearerr will do nothing and in the alternative branch clearerr is
already there.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1706039&group_id=5470
_______________________________________________
Patches mailing list
Patches@python.org
http://mail.python.org/mailman/listinfo/patches
