| msg27975 - (view) |
Author: Matt Fleming (splitscreen) |
Date: 2006-03-31 15:02 |
| Python does not check for POSIX-compliant modes when passing them to open() and fopen(). According to the POSIX standard all modes should start with either an 'a', 'r' or 'w'. This patch implements this check in the check_the_mode() function of fileobject.c and a test has been modified in the test_file.py test. |
|
|
| msg27976 - (view) |
Author: Tim Peters (tim.peters) *  |
Date: 2006-03-31 18:03 |
| Logged In: YES user_id=31435 There's a small danger of backward-incompatibility here, since platforms are free to support any goofy mode characters they like, and Python just passes on whatever the user typed. I understand that some MS compilers in debug mode raise internal exceptions, but that's an MS bug and the workaround is dead easy ("don't do that"). All that said, I'm in favor of this patch ;-). However, if it goes in two things are needed: 1. The error message should be more informative, like PyErr_Format(PyExc_ValueError, "mode argument must " "begin with 'w', 'r', or 'a', not '%.200s'", mode); 2. The Python docs should change to match (i.e., the manual should document this new restriction on mode strings). WRT the test case, it's more natural to write, e.g., TESTFN in s than s.find(TESTFN) != -1 |
|
|
| msg27977 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-04-01 07:08 |
| Logged In: YES user_id=849994 Instead of this patch, I'd rather like check_the_mode to * remove any 'U' from the mode string * if 'U' was found: * error out if the new string begins with 'w' or 'a' * add a 'r' and 'b' if it isn't already given * if not: * error out if the string doesn't begin with 'w', 'r', 'a' What do you think of it? |
|
|
| msg27978 - (view) |
Author: Matt Fleming (splitscreen) |
Date: 2006-04-01 14:14 |
| Logged In: YES user_id=1126061 That seems sensible. What does a mode containing 'U' mean anyway? |
|
|
| msg27979 - (view) |
Author: Matt Fleming (splitscreen) |
Date: 2006-04-01 19:32 |
| Logged In: YES user_id=1126061 Ignore my question, it's for "universal newlines", right? Have I understood your proposal correctly in that you want to remove the 'U' from the mode? |
|
|
| msg27980 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-04-01 20:43 |
| Logged In: YES user_id=849994 Yes, I want to remove 'U' from the mode since it's at this point already recognized by Python, and it's not meaningful to the underlying C library. |
|
|
| msg27981 - (view) |
Author: Matt Fleming (splitscreen) |
Date: 2006-04-01 21:59 |
| Logged In: YES user_id=1126061 Ok, uploading latest patch, not quite sure I've hit the mark here. Please review. |
|
|
| msg27982 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-04-06 08:15 |
| Logged In: YES user_id=849994 splitscreen: your patch was incomplete and could have overwritten memory. tim_one: Attaching new patch implementing what I proposed in my comment below. |
|
|
| msg27983 - (view) |
Author: Matt Fleming (splitscreen) |
Date: 2006-04-12 10:14 |
| Logged In: YES user_id=1126061 Yeah, your patch looks much better. |
|
|
| msg27984 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2006-04-24 01:19 |
| Logged In: YES user_id=31435 This is still ;-) fine with me. There doesn't seem to be a test in fileobject2.diff. WRT the docs, "must either be" should say "must be one of" instead ("either" is proper for two choices, but there are three). It should state that this requirement is new in Python 2.5; in LaTeX, something like \versionchanged[Restriction on first letter of mode string introduced]{2.5} |
|
|
| msg27985 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-04-28 09:55 |
| Logged In: YES user_id=849994 Tim: Did you review file-modes.diff which is really the latest patch? (There's no test in it either, but I'll add one when I check it in) |
|
|
| msg27986 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2006-05-10 02:16 |
| Logged In: YES user_id=31435 You're right, I reviewed the older patch. One of the same comments, though, plus a new one: 1. The docs should state that this requirement is new in Python 2.5; in LaTeX, something like \versionchanged[Restriction on first letter of mode string introduced]{2.5} 2. We can never assume that allocating memory will work: after newmode = PyMem_MALLOC(strlen(mode) + 3); you must guard against `newmode` being NULL (and raise an exception it if is; e.g., call PyErr_NoMemory()). |
|
|
| msg27987 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-05-18 07:01 |
| Logged In: YES user_id=849994 Corrected and committed in rev. 46037. |
|
|