Patches item #1552731, was opened at 2006-09-05 07:47
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1552731&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: Python 2.5
Status: Open
Resolution: Accepted
Priority: 7
Submitted By: Raymond Hettinger (rhettinger)
Assigned to: Raymond Hettinger (rhettinger)
Summary: Fix error checks and leaks in setobject.c
Initial Comment:
Check return values for errors.
Fix refcounts on the error returns;
----------------------------------------------------------------------
>Comment By: Neal Norwitz (nnorwitz)
Date: 2006-09-07 00:05
Message:
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. ;-)
----------------------------------------------------------------------
Comment By: Georg Brandl (gbrandl)
Date: 2006-09-06 00:09
Message:
Logged In: YES
user_id=849994
That misses the possibility of set_contains_key's return
value being 0.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2006-09-06 00:04
Message:
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.
----------------------------------------------------------------------
Comment By: Georg Brandl (gbrandl)
Date: 2006-09-05 23:54
Message:
Logged In: YES
user_id=849994
Shouldn't this go into 2.5 final?
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1552731&group_id=5470
_______________________________________________
Patches mailing list
Patches@python.org
http://mail.python.org/mailman/listinfo/patches
