Skip to content

Conversation

@shihai1991
Copy link
Member

@shihai1991shihai1991 commented Feb 23, 2020

@codecov
Copy link

codecovbot commented Feb 23, 2020

Codecov Report

Merging #18613 into master will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #18613 +/- ## ========================================== + Coverage 82.06% 82.13% +0.06%  ========================================== Files 1955 1955 Lines 584080 584621 +541 Branches 44458 44484 +26 ========================================== + Hits 479327 480176 +849 + Misses 95124 94794 -330 - Partials 9629 9651 +22 
Impacted FilesCoverage Δ
Lib/collections/__init__.py88.72% <0.00%> (-1.71%)⬇️
Lib/idlelib/colorizer.py85.78% <0.00%> (-1.02%)⬇️
Lib/test/test_capi.py88.70% <0.00%> (-0.73%)⬇️
Lib/test/test_asyncio/test_subprocess.py92.06% <0.00%> (-0.43%)⬇️
Objects/listobject.c90.77% <0.00%> (-0.29%)⬇️
Objects/stringlib/codecs.h97.33% <0.00%> (-0.27%)⬇️
Modules/clinic/mathmodule.c.h85.58% <0.00%> (-0.13%)⬇️
Lib/test/test_buffer.py95.14% <0.00%> (-0.11%)⬇️
Modules/_pickle.c74.66% <0.00%> (-0.09%)⬇️
Objects/dictobject.c86.97% <0.00%> (-0.05%)⬇️
... and 48 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbe2e0b...a2f1f99. Read the comment docs.

binascii_clear(PyObject*m)
{
binascii_state*state=get_binascii_state(m);
if (state==NULL){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that state can be NULL. If it's NULL, it's a bug in Python. No? Same remark in binascii_traverse().

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do some debug in my vm:

Breakpoint 1, binascii_traverse (module=0x7ffff7e803d0, visit=0x4630b6 <bad_traverse_test>, arg=0x0) at /temp/shihai/cpython/Modules/binascii.c:1654 1654{(gdb) n 1655 binascii_state *state = get_binascii_state(module); (gdb) n 1656 if (state == NULL){(gdb) n 1657 return -1; (gdb) p state $5 = (binascii_state *) 0x0 (gdb) c Continuing. Breakpoint 3, binascii_exec (module=0x7ffff7e803d0) at /temp/shihai/cpython/Modules/binascii.c:1615 1615 binascii_exec(PyObject *module){

What's the reason?
the malloc operation of md_state will be called after first calling binascii_traverse.
https://github.com/python/cpython/blob/master/Objects/moduleobject.c#L399.

};

staticint
binascii_traverse(PyObject*m, visitprocvisit, void*arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I would prefer to use longer variable name than just "m". Rename "m" to "mod" or "module". Same remark if following functions.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, no probleam, i will update it.

binascii_traverse(PyObject*m, visitprocvisit, void*arg)
{
binascii_state*state=get_binascii_state(m);
if (state==NULL){
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update this behavior soon.

@vstinner
Copy link
Member

I created https://bugs.python.org/issue39824 to change the md_state==NULL case.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@shihai1991@vstinner@the-knights-who-say-ni@bedevere-bot