Re: [Python-checkins] CVS: python/dist/src/Lib/curses wrapper.py,1.2,1.3
I just don't buy this. Look at that code. Look at the definition of finally. That code simply screams for the use of finally. Eric: just what happens? What is "broke things"? Do you have a reproducible test case that I can use? In good conscience, I have to disagree with reverting this stuff. If we're seeking to make the code the best possible, then this is a backward step. Besides its redundancy, it also places the traceback into a local variable and then raises an exception -- a perfect recipe for creating a ref loop. Cheers, -g On Mon, Jun 26, 2000 at 05:50:42PM -0700, A.M. Kuchling wrote:
Update of /cvsroot/python/python/dist/src/Lib/curses In directory slayer.i.sourceforge.net:/tmp/cvs-serv26153
Modified Files: wrapper.py Log Message: Drop back to old version of wrapper(); ESR reports that it broke things, and I lack the time to track down the cause.
Index: wrapper.py =================================================================== RCS file: /cvsroot/python/python/dist/src/Lib/curses/wrapper.py,v retrieving revision 1.2 retrieving revision 1.3 diff -C2 -r1.2 -r1.3 *** wrapper.py 2000/06/10 23:39:05 1.2 --- wrapper.py 2000/06/27 00:50:40 1.3 *************** *** 18,24 **** --- 18,26 ---- """
+ res = None try: # Initialize curses stdscr=curses.initscr() + # Turn off echoing of keys, and enter cbreak mode, # where no buffering is performed on keyboard input *************** *** 30,39 **** stdscr.keypad(1)
! return apply(func, (stdscr,) + rest) ! ! finally: ! # Restore the terminal to a sane state on the way out. stdscr.keypad(0) curses.echo() ; curses.nocbreak() curses.endwin()
--- 32,51 ---- stdscr.keypad(1)
! res = apply(func, (stdscr,) + rest) ! except: ! # In the event of an error, restore the terminal ! # to a sane state. stdscr.keypad(0) curses.echo() ; curses.nocbreak() curses.endwin() + + # Pass the exception upwards + (exc_type, exc_value, exc_traceback) = sys.exc_info() + raise exc_type, exc_value, exc_traceback + else: + # Set everything back to normal + stdscr.keypad(0) + curses.echo() ; curses.nocbreak() + curses.endwin() # Terminate curses
+ return res
_______________________________________________ Python-checkins mailing list Python-checkins@python.org http://www.python.org/mailman/listinfo/python-checkins
-- Greg Stein, http://www.lyra.org/
Besides its redundancy, it also places the traceback into a local variable and then raises an exception -- a perfect recipe for creating a ref loop.
Isnt it true that _every_ time you store a traceback object as a local variable, and that traceback has a reference to the frame holding the local, you _always_ get a reference loop, regardless of how you exit the function? ie, I believe that _every_ time you store a traceback obtained from inside an except handler, you _always_ get a ref-loop, period. The only way to clean the cycle is to explictly unbind the local from the traceback. Mark.
On Wed, Jun 28, 2000 at 12:45:07AM +1000, Mark Hammond quoted:
Besides its redundancy, it also places the traceback into a local variable and then raises an exception -- a perfect recipe for creating a ref loop.
I think this is irrelevant for the curses.wrapper() function, which simply restores the terminal state before re-raising the exception. It probably won't ever be called repeatedly, making the question of a memory leak moot. Yes, I'd like to have wrapper() use Greg Stein's simpler and tidier code. Yes, I'd like to know why ESR claims the simpler version doesn't work and have a replicable test case. But right now there are higher priority things to do, so I'm shelving this until after 1.6b1. (Right now I'm trying to get the curses module compiling on Tru64, and things don't look good.) --amk
Greg Stein <gstein@lyra.org>:
I just don't buy this.
Look at that code. Look at the definition of finally. That code simply screams for the use of finally.
Eric: just what happens? What is "broke things"? Do you have a reproducible test case that I can use?
What happened is that your version failed to pass an error exception upwards after fielding it. My test case was a buggy version of cmlconfigure.py, bug since fixed. Cleanliness is good, but the code has to work first. -- <a href="http://www.tuxedo.org/~esr">Eric S. Raymond</a> Faith may be defined briefly as an illogical belief in the occurrence of the improbable...A man full of faith is simply one who has lost (or never had) the capacity for clear and realistic thought. He is not a mere ass: he is actually ill. -- H. L. Mencken
Eric S. Raymond wrote:
What happened is that your version failed to pass an error exception upwards after fielding it. My test case was a buggy version of cmlconfigure.py, bug since fixed.
Cleanliness is good, but the code has to work first.
umm. as far as I can tell from the patch, there's no way your code can work in a situation where try/finally wouldn't do exactly the same thing. if you *do* get different results (try inserting "raise whatever" in the try clause to test this), it might be that you've stumbled upon a real bug (either in Python or in the curses C bindings)... </F>
participants (5)
-
Andrew Kuchling -
Eric S. Raymond -
Fredrik Lundh -
Greg Stein -
Mark Hammond