| msg51074 - (view) |
Author: Raymond Hettinger (rhettinger) *  |
Date: 2006-09-05 14:47 |
| Check return values for errors. Fix refcounts on the error returns; |
|
|
| msg51075 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-09-06 06:54 |
| Logged In: YES user_id=849994 Shouldn't this go into 2.5 final? |
|
|
| msg51076 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2006-09-06 07:04 |
| Logged In: YES user_id=33168 This is fine. Though I wonder if hunks like this: - if (set_contains_key(so, key)) { + int rv = set_contains_key(so, key); + if (rv == -1) { + Py_DECREF(it); + Py_DECREF(result); + Py_DECREF(key); + return NULL; + } + if (rv) { if (set_add_key(result, key) == -1) { would be clearer (to me at least) more like: if (set_contains_key(so, key) == -1 | |
set_add_key(result, key) == -1) { That ensures the cleanup is the same. There are several similar hunks. |
|
| msg51077 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-09-06 07:09 |
| Logged In: YES user_id=849994 That misses the possibility of set_contains_key's return value being 0. |
|
|
| msg51078 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2006-09-07 07:05 |
| Logged In: YES user_id=33168 rev 51798 for 2.6 was checked in. Georg, true. I was thinking that something like this would be nicer: int rv = set_contains_key(so, key); if (rv == -1 | |
(rv != 0 && set_add_key(result, key) == -1)) { I figured the transformation would be easy. But now that I look at the code above, I really don't like that at all. The only other solution I can think of is to use a goto: int rv = set_contains_key(so, key); if (rv == -1) goto some_error; if (rv) { if (set_add_key(result, key) == -1) { some_error: I'm not really sure I like that either. So basically no matter which way the code looks, I'm not gonna be happy. :-) Oh well. Raymond, were you planning on backporting this or did you want someone else to? I volunteer to backport to 2.4. ;-) |
|
| msg51079 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2006-09-07 20:13 |
| Logged In: YES user_id=80475 Georg, the buildbot seems to be happy with this patch as applied to the head, would you please backport it to the Py2.5 release for me (since I'm away from my svn commit machine for a while). Thanks, Raymond |
|
|
| msg51080 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2006-09-08 06:03 |
| Logged In: YES user_id=849994 Sure. Committed revision 51825 for 2.5 branch. |
|
|