Skip to content

Commit e80b8b3

Browse files
committed
Address Victors review
1 parent 4bbaea2 commit e80b8b3

2 files changed

Lines changed: 25 additions & 14 deletions

File tree

Lib/test/test_capi/test_dict.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,13 @@ def test_dict_as_frozendict_and_clear(self):
625625
self.assertEqual(f, frozendict())
626626
self.assertIs(type(d), dict)
627627
self.assertEqual(d, {})
628-
# CRASHES check([])
629-
# CRASHES check(frozendict())
630-
# CRASHES check(NULL)
628+
629+
class DictSubtype(dict): ...
630+
631+
for wrong_input in (frozendict(), DictSubtype(), [], None):
632+
with self.subTest(wrong_input=wrong_input):
633+
with self.assertRaises(SystemError):
634+
as_frozendict(wrong_input)
631635

632636
def test_frozendict_check(self):
633637
# Test PyFrozenDict_Check()

Objects/dictobject.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8532,33 +8532,40 @@ frozendict_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
85328532
static void
85338533
transfer_keys_and_values_lock_held(PyObject *res, PyObject *dict)
85348534
{
8535-
85368535
PyDictObject *new = (PyDictObject *)res;
85378536
PyDictObject *old = (PyDictObject *)dict;
8537+
assert(can_modify_dict(old));
8538+
85388539
// Fast path: do nothing on an empty dict:
85398540
if (old->ma_keys == Py_EMPTY_KEYS) {
85408541
return;
85418542
}
85428543

8543-
// Transfer keys and values from dict to res:
8544-
STORE_USED(new, old->ma_used);
8545-
set_keys(new, old->ma_keys);
8546-
set_values(new, old->ma_values);
8547-
ASSERT_CONSISTENT(new);
8544+
Py_ssize_t used = old->ma_used;
8545+
PyDictKeysObject *keys = old->ma_keys;
8546+
PyDictValues *values = old->ma_values;
85488547

8549-
// Now, clear the old dict keys and values, but do not decref them:
8548+
// Clear the old dict keys and values, but do not decref them:
85508549
clear_common((PyDictObject *)dict);
85518550
set_keys(old, Py_EMPTY_KEYS);
85528551
set_values(old, NULL);
8553-
ASSERT_CONSISTENT(dict);
8552+
ASSERT_CONSISTENT(old);
8553+
8554+
// Transfer keys and values from dict to frozendict:
8555+
new->ma_used = used;
8556+
new->ma_keys = keys;
8557+
new->ma_values = values;
8558+
ASSERT_CONSISTENT(new);
85548559
}
85558560

85568561
PyObject *
85578562
PyDict_AsFrozenDictAndClear(PyObject *dict)
85588563
{
8559-
assert(dict != NULL);
8560-
assert(PyDict_CheckExact(dict));
8561-
assert(can_modify_dict((PyDictObject *)dict));
8564+
if (dict == NULL || !PyDict_CheckExact(dict)) {
8565+
PyErr_BadInternalCall();
8566+
return NULL;
8567+
}
8568+
85628569
PyObject *res = frozendict_new_untracked(&PyFrozenDict_Type);
85638570
if (res == NULL) {
85648571
return NULL;

0 commit comments

Comments
 (0)