[Python-Dev] Patch #1731330 - pysqlite_cache_display - missing Py_DECREF
I've added patch #1731330 to fix a missing Py_DECREF in pysqlite_cache_display. I've attached the diff to this email. I haven't actually been able to test this - haven't been able to get pysqlite compiled here on cygwin yet. I just noticed it when taking an example of using PyObject_Print ... Cheers, Tim Delaney sqlite_cache.diff Description: Binary data ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
On 6/1/07, Fred L. Drake, Jr. [EMAIL PROTECTED] wrote: Changes in general are a source of risk; they have to be considered carefully. We've seen too many cases in which a change was thought to be safe, but broke something for someone. Avoiding style-only changes helps avoid introducing problems without being able to predict them; there are tests for ConfigParser, but it's hard to be sure every corner case has been covered. I understand what you mean, all changes carry a certain risk. Especially in code that is so widely relied upon as the Standard Library. But the alternative, which is to let the code rot, while one-line fixes are applied upon it, is a much worse alternative. It is true that unit tests does not cover all corner cases and that you can't be 100% sure that a change won't break something for someone. But on the other hand, the whole point with unit tests is to facilitate exactly these kind of changes. If something breaks then that is a great opportunity to introduce more tests. This is a general policy in the Python project, not simply my preference. I'd love to be able to say yes, the code is painful to read, let's make it nicer, but it's hard to say that without being able to say I'm sure it won't break anything for anybody. Python's too flexible for that to be easy. While what you have stated is the policy, I can't help but think that it is totally misguided (no offense intended). Maybe the policy can be reevaluated? -- mvh Björn ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Patch #1731330 - pysqlite_cache_display - missing Py_DECREF
On 6/5/07, Tim Delaney [EMAIL PROTECTED] wrote: I've added patch #1731330 to fix a missing Py_DECREF in pysqlite_cache_display. I've attached the diff to this email. I haven't actually been able to test this - haven't been able to get pysqlite compiled here on cygwin yet. I just noticed it when taking an example of using PyObject_Print ... Committed revision 55783. -- --Guido van Rossum (home page: http://www.python.org/~guido/) ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Patch #1731330 - pysqlite_cache_display - missing Py_DECREF
On 06/06/07, Guido van Rossum [EMAIL PROTECTED] wrote: On 6/5/07, Tim Delaney [EMAIL PROTECTED] wrote: I've added patch #1731330 to fix a missing Py_DECREF in pysqlite_cache_display. I've attached the diff to this email. I haven't actually been able to test this - haven't been able to get pysqlite compiled here on cygwin yet. I just noticed it when taking an example of using PyObject_Print ... Committed revision 55783. Thanks. I've added a comment that it also needs to be applied to p3yk. I've done a quick seach for other places with the same code, on the off-chance that it was copied from elsewhere. Didn't turn up any other cases. Tim Delaney ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] popen test case inquiry in r55735 using PCBuild8
Mark,
My apologies for being a day late, got working on some other things.
So here's the scoop as it relates to the issue at hand:
- If you run rt.bat from the trunk as-is and place it in a path that
contains an empty space, you receive the error outlined in
resultwithspace.txt.
- If you run rt.bat from the trunk as-is and place it in a path that
does not contain an empty space, you receive no errors as outlined in
resultwithoutspace.txt.
- If you run rt.bat with the patch, on Windows XP, you receive no errors
as outlined in resultafterpatch.txt.
The patch is attached. Probably my biggest question now is the use of
GetVersion as opposed to GetVersionEx. According to the MSDN, it doesn't
appear to be all that undesirable:
http://msdn2.microsoft.com/en-us/library/ms724451.aspx
Your thoughts?
Joseph Armbruster
Joseph Armbruster wrote:
Mark,
Sounds good, I will get patching tonight. Any thoughts on CreateProcessW ?
Joseph Armbruster
On 6/4/07, *Mark Hammond* [EMAIL PROTECTED]
mailto:[EMAIL PROTECTED] wrote:
All,
I wanted to pass this one around before opening an issue on it.
When running the unit test for popen via rt.bat (in PCBuild8),
I received the following error:
=== BEGIN ERROR ===
C:\Documents and
Settings\joe\Desktop\Development\Python\trunk\PCbuild8rt test_popen
Deleting .pyc/.pyo files ...
43 .pyc deleted, 0 .pyo deleted
C:\Documents and
Settings\joe\Desktop\Development\Python\trunk\PCbuild8win32Re
lease\python.exe -E -tt ../lib/test/regrtest.py test_popen
test_popen
test test_popen failed -- Traceback (most recent call last):
File C:\Documents and Settings\joe\Desktop\Development\Python\...
I can't reproduce this. I expect you will find it is due to the
space in
the filename of your Python directory, via cmd.exe's documented
behaviour
with quote characters. A patch that allows the test suite to work
in such
an environment would be welcome, but I think you might end up
needing access
to GetShortPathName() rather than CreateProcess().
Cheers,
Mark
Index: posixmodule.c
===
--- posixmodule.c (revision 55784)
+++ posixmodule.c (working copy)
@@ -4793,6 +4793,9 @@
PROCESS_INFORMATION piProcInfo;
STARTUPINFO siStartInfo;
DWORD dwProcessFlags = 0; /* no NEW_CONSOLE by default for Ctrl+C
handling */
+ DWORD dwVersion = 0;
+ DWORD dwMajorVersion = 0;
+ DWORD dwMinorVersion = 0;
char *s1,*s2, *s3 = /c ;
const char *szConsoleSpawn = w9xpopen.exe;
int i;
@@ -4814,8 +4817,20 @@
--comshell;
++comshell;
- if (GetVersion() 0x8000
- _stricmp(comshell, command.com) != 0) {
+ dwVersion = GetVersion();
+ dwMajorVersion = (DWORD)(LOBYTE(LOWORD(dwVersion)));
+ dwMinorVersion = (DWORD)(HIBYTE(LOWORD(dwVersion)));
+
+ if (dwMajorVersion = 5)
+ {
+ /* Current, XP + Vista? */
+ x = i + strlen(s3) + strlen(cmdstring) + 1;
+ s2 = (char *)alloca(x);
+ ZeroMemory(s2, x);
+ PyOS_snprintf(s2, x, %s, cmdstring);
+ }
+ else if (dwMajorVersion == 4
+ _stricmp(comshell, command.com) != 0) {
/* NT/2000 and not using command.com. */
x = i + strlen(s3) + strlen(cmdstring) + 1;
s2 = (char *)alloca(x);
@@ -4836,23 +4851,23 @@
modulepath[x] = '\0';
/* Create the full-name to w9xpopen, so we can test it
exists */
strncat(modulepath,
- szConsoleSpawn,
- (sizeof(modulepath)/sizeof(modulepath[0]))
- -strlen(modulepath));
+ szConsoleSpawn,
+
(sizeof(modulepath)/sizeof(modulepath[0]))
+ -strlen(modulepath));
if (stat(modulepath, statinfo) != 0) {
size_t mplen =
sizeof(modulepath)/sizeof(modulepath[0]);
/* Eeek - file-not-found - possibly an embedding
situation - see if we can locate it in
sys.prefix
*/
strncpy(modulepath,
- Py_GetExecPrefix(),
- mplen);
+ Py_GetExecPrefix(),
+ mplen);
Re: [Python-Dev] popen test case inquiry in r55735 using PCBuild8
My apologies for being a day late, got working on some other things. So here's the scoop as it relates to the issue at hand: - If you run rt.bat from the trunk as-is and place it in a path that contains an empty space, you receive the error outlined in resultwithspace.txt. - If you run rt.bat from the trunk as-is and place it in a path that does not contain an empty space, you receive no errors as outlined in resultwithoutspace.txt. - If you run rt.bat with the patch, on Windows XP, you receive no errors as outlined in resultafterpatch.txt. In that last step, you failed to indicate if the path had a space or not. ie, on Windows XP I get that behaviour now without needing to apply the patch. The patch is attached. The vast majority of the patch is insignificant - it is either adding braces where they are not necessary, or changing whitespace inappropriately (the spaces you replaced are so the lines all line up regardless of the tab width.) It seems there is only one significant block in your patch, and its not clear to me what the intent of the patch is - I admit I didn't apply it and look at it in-place, but a couple of comments indicating exactly what you are trying to do would be good, especially as I'm not aware of this behaviour change from Win2K - WinXP. Probably my biggest question now is the use of GetVersion as opposed to GetVersionEx. The existing code explicitly checks if it is the 9x or NT family, which your patch no longer does. It seems to me that Windows ME will also qualify - although in general the strcmp for command.com will succeed, if an alternative shell is installed on a ME box it will do the wrong thing. If you need to check anything more than the high-bit of GetVersion(), IMO it should be replaced with GetVersionEx(). Cheers, Mark ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] popen test case inquiry in r55735 using PCBuild8
My apologies for being a day late, got working on some other things. So here's the scoop as it relates to the issue at hand: Something else I meant to mention: your problem is that the test suite fails in some circumstances, but these circumstances are not met for most core developers or when running the python test suite from the directory it is built in, but your proposed fix is a patch to os.popen(). There would also need to be new test cases added to demonstrate this bug in a normal test run. Cheers, Mark ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
