Skip to content

Commit b315383

Browse files
Yhg1sYhg1s
committed
Clean up _bsddb.c: add a couple dozen missing Py_DECREF()'s, a handful of
missing PyObject_Del()'s, simplify some code by using Py_BuildValue() instead of creating a tuple with items manually, stop clobbering builtin exceptions in a few places, and guard against NULL-returning functions some more. This fixes 117 of the 780 (!?!#%@#$!!) reference leaks in test_bsddb3. I ain't not done yet, although this review of 5kloc was just the easy part.
1 parent e920f0d commit b315383

1 file changed

Lines changed: 73 additions & 65 deletions

File tree

‎Modules/_bsddb.c‎

Lines changed: 73 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,7 @@ newDBObject(DBEnvObject* arg, int flags)
779779
Py_DECREF(self->myenvobj);
780780
self->myenvobj = NULL;
781781
}
782+
PyObject_Del(self);
782783
self = NULL;
783784
}
784785
return self;
@@ -895,6 +896,7 @@ newDBEnvObject(int flags)
895896
err = db_env_create(&self->db_env, flags);
896897
MYDB_END_ALLOW_THREADS;
897898
if (makeDBError(err)) {
899+
PyObject_Del(self);
898900
self = NULL;
899901
}
900902
else {
@@ -1001,6 +1003,7 @@ newDBLockObject(DBEnvObject* myenv, u_int32_t locker, DBT* obj,
10011003
#endif
10021004
MYDB_END_ALLOW_THREADS;
10031005
if (makeDBError(err)) {
1006+
PyObject_Del(self);
10041007
self = NULL;
10051008
}
10061009

@@ -1067,26 +1070,20 @@ _db_associateCallback(DB* db, const DBT* priKey, const DBT* priData,
10671070
DBObject* secondaryDB = (DBObject*)db->app_private;
10681071
PyObject* callback = secondaryDB->associateCallback;
10691072
int type = secondaryDB->primaryDBType;
1070-
PyObject* key;
1071-
PyObject* data;
10721073
PyObject* args;
10731074
PyObject* result = NULL;
10741075

10751076

10761077
if (callback != NULL) {
10771078
MYDB_BEGIN_BLOCK_THREADS;
10781079

1079-
if (type == DB_RECNO || type == DB_QUEUE) {
1080-
key = PyInt_FromLong( *((db_recno_t*)priKey->data));
1081-
}
1082-
else {
1083-
key = PyString_FromStringAndSize(priKey->data, priKey->size);
1084-
}
1085-
data = PyString_FromStringAndSize(priData->data, priData->size);
1086-
args = PyTuple_New(2);
1080+
if (type == DB_RECNO || type == DB_QUEUE)
1081+
args = Py_BuildValue("(ls#)", *((db_recno_t*)priKey->data),
1082+
priData->data, priData->size);
1083+
else
1084+
args = Py_BuildValue("(s#s#)", priKey->data, priKey->size,
1085+
priData->data, priData->size);
10871086
if (args != NULL) {
1088-
PyTuple_SET_ITEM(args, 0, key); /* steals reference */
1089-
PyTuple_SET_ITEM(args, 1, data); /* steals reference */
10901087
result = PyEval_CallObject(callback, args);
10911088
}
10921089
if (args == NULL || result == NULL) {
@@ -1130,10 +1127,8 @@ _db_associateCallback(DB* db, const DBT* priKey, const DBT* priData,
11301127
PyErr_Print();
11311128
}
11321129

1133-
Py_DECREF(args);
1134-
if (result) {
1135-
Py_DECREF(result);
1136-
}
1130+
Py_XDECREF(args);
1131+
Py_XDECREF(result);
11371132

11381133
MYDB_END_BLOCK_THREADS;
11391134
}
@@ -1187,7 +1182,7 @@ DB_associate(DBObject* self, PyObject* args, PyObject* kwargs)
11871182

11881183
/* Save a reference to the callback in the secondary DB. */
11891184
Py_XDECREF(secondaryDB->associateCallback);
1190-
Py_INCREF(callback);
1185+
Py_XINCREF(callback);
11911186
secondaryDB->associateCallback = callback;
11921187
secondaryDB->primaryDBType = _DB_get_type(self);
11931188

@@ -1542,6 +1537,7 @@ DB_pget(DBObject* self, PyObject* args, PyObject* kwargs)
15421537
#else
15431538
retval = Py_BuildValue("OOO", keyObj, pkeyObj, dataObj);
15441539
#endif
1540+
Py_DECREF(keyObj);
15451541
}
15461542
else /* return just the pkey and data */
15471543
{
@@ -1551,6 +1547,8 @@ DB_pget(DBObject* self, PyObject* args, PyObject* kwargs)
15511547
retval = Py_BuildValue("OO", pkeyObj, dataObj);
15521548
#endif
15531549
}
1550+
Py_DECREF(dataObj);
1551+
Py_DECREF(pkeyObj);
15541552
FREE_DBT(pkey);
15551553
FREE_DBT(data);
15561554
}
@@ -1733,6 +1731,10 @@ DB_join(DBObject* self, PyObject* args)
17331731
cursors[length] = NULL;
17341732
for (x=0; x<length; x++) {
17351733
PyObject* item = PySequence_GetItem(cursorsObj, x);
1734+
if (item == NULL) {
1735+
free(cursors);
1736+
return NULL;
1737+
}
17361738
if (!DBCursorObject_Check(item)) {
17371739
PyErr_SetString(PyExc_TypeError,
17381740
"Sequence of DBCursor objects expected");
@@ -1843,8 +1845,10 @@ DB_open(DBObject* self, PyObject* args, PyObject* kwargs)
18431845
#endif
18441846

18451847
if (NULL == self->db) {
1846-
PyErr_SetObject(DBError, Py_BuildValue("(is)", 0,
1847-
"Cannot call open() twice for DB object"));
1848+
PyObject *t = Py_BuildValue("(is)", 0,
1849+
"Cannot call open() twice for DB object");
1850+
PyErr_SetObject(DBError, t);
1851+
Py_DECREF(t);
18481852
return NULL;
18491853
}
18501854

@@ -2014,8 +2018,6 @@ _db_compareCallback(DB* db,
20142018
int res = 0;
20152019
PyObject *args;
20162020
PyObject *result;
2017-
PyObject *leftObject;
2018-
PyObject *rightObject;
20192021
DBObject *self = (DBObject *)db->app_private;
20202022

20212023
if (self == NULL || self->btCompareCallback == NULL) {
@@ -2031,14 +2033,11 @@ _db_compareCallback(DB* db,
20312033
} else {
20322034
MYDB_BEGIN_BLOCK_THREADS;
20332035

2034-
leftObject = PyString_FromStringAndSize(leftKey->data, leftKey->size);
2035-
rightObject = PyString_FromStringAndSize(rightKey->data, rightKey->size);
2036-
2037-
args = PyTuple_New(2);
2036+
args = Py_BuildValue("s#s#", leftKey->data, leftKey->size,
2037+
rightKey->data, rightKey->size);
20382038
if (args != NULL) {
2039+
/* XXX(twouters) I highly doubt this INCREF is correct */
20392040
Py_INCREF(self);
2040-
PyTuple_SET_ITEM(args, 0, leftObject); /* steals reference */
2041-
PyTuple_SET_ITEM(args, 1, rightObject); /* steals reference */
20422041
result = PyEval_CallObject(self->btCompareCallback, args);
20432042
}
20442043
if (args == NULL || result == NULL) {
@@ -2055,7 +2054,7 @@ _db_compareCallback(DB* db,
20552054
res = _default_cmp(leftKey, rightKey);
20562055
}
20572056

2058-
Py_DECREF(args);
2057+
Py_XDECREF(args);
20592058
Py_XDECREF(result);
20602059

20612060
MYDB_END_BLOCK_THREADS;
@@ -2068,7 +2067,7 @@ DB_set_bt_compare(DBObject* self, PyObject* args)
20682067
{
20692068
int err;
20702069
PyObject *comparator;
2071-
PyObject *tuple, *emptyStr, *result;
2070+
PyObject *tuple, *result;
20722071

20732072
if (!PyArg_ParseTuple(args, "O:set_bt_compare", &comparator))
20742073
return NULL;
@@ -2085,17 +2084,12 @@ DB_set_bt_compare(DBObject* self, PyObject* args)
20852084
* string objects here. verify that it returns an int (0).
20862085
* err if not.
20872086
*/
2088-
tuple = PyTuple_New(2);
2089-
emptyStr = PyString_FromStringAndSize(NULL, 0);
2090-
if (tuple == NULL || emptyStr == NULL)
2091-
return NULL;
2092-
2093-
Py_INCREF(emptyStr); /* now we have two references */
2094-
PyTuple_SET_ITEM(tuple, 0, emptyStr); /* steals reference */
2095-
PyTuple_SET_ITEM(tuple, 1, emptyStr); /* steals reference */
2087+
tuple = Py_BuildValue("(ss)", "", "");
20962088
result = PyEval_CallObject(comparator, tuple);
20972089
Py_DECREF(tuple);
2098-
if (result == NULL || !PyInt_Check(result)) {
2090+
if (result == NULL)
2091+
return NULL;
2092+
if (!PyInt_Check(result)) {
20992093
PyErr_SetString(PyExc_TypeError,
21002094
"callback MUST return an int");
21012095
return NULL;
@@ -2104,6 +2098,7 @@ DB_set_bt_compare(DBObject* self, PyObject* args)
21042098
"callback failed to return 0 on two empty strings");
21052099
return NULL;
21062100
}
2101+
Py_DECREF(result);
21072102

21082103
/* We don't accept multiple set_bt_compare operations, in order to
21092104
* simplify the code. This would have no real use, as one cannot
@@ -2122,9 +2117,7 @@ DB_set_bt_compare(DBObject* self, PyObject* args)
21222117
PyEval_InitThreads();
21232118
#endif
21242119

2125-
err = self->db->set_bt_compare(self->db,
2126-
(comparator != NULL ?
2127-
_db_compareCallback : NULL));
2120+
err = self->db->set_bt_compare(self->db, _db_compareCallback);
21282121

21292122
if (err) {
21302123
/* restore the old state in case of error */
@@ -2621,8 +2614,9 @@ Py_ssize_t DB_length(DBObject* self)
26212614
void* sp;
26222615

26232616
if (self->db == NULL) {
2624-
PyErr_SetObject(DBError,
2625-
Py_BuildValue("(is)", 0, "DB object has been closed"));
2617+
PyObject *t = Py_BuildValue("(is)", 0, "DB object has been closed");
2618+
PyErr_SetObject(DBError, t);
2619+
Py_DECREF(t);
26262620
return -1;
26272621
}
26282622

@@ -2698,8 +2692,9 @@ DB_ass_sub(DBObject* self, PyObject* keyobj, PyObject* dataobj)
26982692
int flags = 0;
26992693

27002694
if (self->db == NULL) {
2701-
PyErr_SetObject(DBError,
2702-
Py_BuildValue("(is)", 0, "DB object has been closed"));
2695+
PyObject *t = Py_BuildValue("(is)", 0, "DB object has been closed");
2696+
PyErr_SetObject(DBError, t);
2697+
Py_DECREF(t);
27032698
return -1;
27042699
}
27052700

@@ -2798,16 +2793,17 @@ _DB_make_list(DBObject* self, DB_TXN* txn, int type)
27982793
return NULL;
27992794

28002795
list = PyList_New(0);
2801-
if (list == NULL) {
2802-
PyErr_SetString(PyExc_MemoryError, "PyList_New failed");
2796+
if (list == NULL)
28032797
return NULL;
2804-
}
28052798

28062799
/* get a cursor */
28072800
MYDB_BEGIN_ALLOW_THREADS;
28082801
err = self->db->cursor(self->db, txn, &cursor, 0);
28092802
MYDB_END_ALLOW_THREADS;
2810-
RETURN_IF_ERR();
2803+
if (makeDBError(err)) {
2804+
Py_DECREF(list);
2805+
return NULL;
2806+
}
28112807

28122808
if (CHECK_DBFLAG(self, DB_THREAD)) {
28132809
key.flags = DB_DBT_REALLOC;
@@ -2858,10 +2854,13 @@ _DB_make_list(DBObject* self, DB_TXN* txn, int type)
28582854
break;
28592855
}
28602856
break;
2857+
default:
2858+
PyErr_Format(PyExc_ValueError, "Unknown key type 0x%x", type);
2859+
item = NULL;
2860+
break;
28612861
}
28622862
if (item == NULL) {
28632863
Py_DECREF(list);
2864-
PyErr_SetString(PyExc_MemoryError, "List item creation failed");
28652864
list = NULL;
28662865
goto done;
28672866
}
@@ -3199,6 +3198,7 @@ DBC_pget(DBCursorObject* self, PyObject* args, PyObject *kwargs)
31993198
#else
32003199
retval = Py_BuildValue("OOO", keyObj, pkeyObj, dataObj);
32013200
#endif
3201+
Py_DECREF(keyObj);
32023202
FREE_DBT(key);
32033203
}
32043204
else /* return just the pkey and data */
@@ -3209,6 +3209,8 @@ DBC_pget(DBCursorObject* self, PyObject* args, PyObject *kwargs)
32093209
retval = Py_BuildValue("OO", pkeyObj, dataObj);
32103210
#endif
32113211
}
3212+
Py_DECREF(dataObj);
3213+
Py_DECREF(pkeyObj);
32123214
FREE_DBT(pkey);
32133215
FREE_DBT(data);
32143216
}
@@ -4384,18 +4386,14 @@ DBEnv_log_archive(DBEnvObject* self, PyObject* args)
43844386
RETURN_IF_ERR();
43854387

43864388
list = PyList_New(0);
4387-
if (list == NULL) {
4388-
PyErr_SetString(PyExc_MemoryError, "PyList_New failed");
4389+
if (list == NULL)
43894390
return NULL;
4390-
}
43914391

43924392
if (log_list) {
43934393
for (log_list_start = log_list; *log_list != NULL; ++log_list) {
43944394
item = PyString_FromString (*log_list);
43954395
if (item == NULL) {
43964396
Py_DECREF(list);
4397-
PyErr_SetString(PyExc_MemoryError,
4398-
"List item creation failed");
43994397
list = NULL;
44004398
break;
44014399
}
@@ -4492,8 +4490,10 @@ DBTxn_commit(DBTxnObject* self, PyObject* args)
44924490
return NULL;
44934491

44944492
if (!self->txn) {
4495-
PyErr_SetObject(DBError, Py_BuildValue("(is)", 0,
4496-
"DBTxn must not be used after txn_commit or txn_abort"));
4493+
PyObject *t = Py_BuildValue("(is)", 0, "DBTxn must not be used "
4494+
"after txn_commit or txn_abort");
4495+
PyErr_SetObject(DBError, t);
4496+
Py_DECREF(t);
44974497
return NULL;
44984498
}
44994499
txn = self->txn;
@@ -4527,8 +4527,10 @@ DBTxn_prepare(DBTxnObject* self, PyObject* args)
45274527
}
45284528

45294529
if (!self->txn) {
4530-
PyErr_SetObject(DBError, Py_BuildValue("(is)", 0,
4531-
"DBTxn must not be used after txn_commit or txn_abort"));
4530+
PyObject *t = Py_BuildValue("(is)", 0,"DBTxn must not be used "
4531+
"after txn_commit or txn_abort");
4532+
PyErr_SetObject(DBError, t);
4533+
Py_DECREF(t);
45324534
return NULL;
45334535
}
45344536
MYDB_BEGIN_ALLOW_THREADS;
@@ -4547,8 +4549,10 @@ DBTxn_prepare(DBTxnObject* self, PyObject* args)
45474549
return NULL;
45484550

45494551
if (!self->txn) {
4550-
PyErr_SetObject(DBError, Py_BuildValue("(is)", 0,
4551-
"DBTxn must not be used after txn_commit or txn_abort"));
4552+
PyObject *t = Py_BuildValue("(is)", 0, "DBTxn must not be used "
4553+
"after txn_commit or txn_abort");
4554+
PyErr_SetObject(DBError, t);
4555+
Py_DECREF(t);
45524556
return NULL;
45534557
}
45544558
MYDB_BEGIN_ALLOW_THREADS;
@@ -4570,8 +4574,10 @@ DBTxn_abort(DBTxnObject* self, PyObject* args)
45704574
return NULL;
45714575

45724576
if (!self->txn) {
4573-
PyErr_SetObject(DBError, Py_BuildValue("(is)", 0,
4574-
"DBTxn must not be used after txn_commit or txn_abort"));
4577+
PyObject *t = Py_BuildValue("(is)", 0, "DBTxn must not be used "
4578+
"after txn_commit or txn_abort");
4579+
PyErr_SetObject(DBError, t);
4580+
Py_DECREF(t);
45754581
return NULL;
45764582
}
45774583
txn = self->txn;
@@ -4597,8 +4603,10 @@ DBTxn_id(DBTxnObject* self, PyObject* args)
45974603
return NULL;
45984604

45994605
if (!self->txn) {
4600-
PyErr_SetObject(DBError, Py_BuildValue("(is)", 0,
4601-
"DBTxn must not be used after txn_commit or txn_abort"));
4606+
PyObject *t = Py_BuildValue("(is)", 0, "DBTxn must not be used "
4607+
"after txn_commit or txn_abort");
4608+
PyErr_SetObject(DBError, t);
4609+
Py_DECREF(t);
46024610
return NULL;
46034611
}
46044612
MYDB_BEGIN_ALLOW_THREADS;

0 commit comments

Comments
 (0)