Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Increase ob's ref count in tp_repr to avoid accidental free#1160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
lostmsu merged 1 commit intopythonnet:masterfromDanBarzilian:PR/fix-tp_repr-bug
Jun 17, 2020
Merged

Increase ob's ref count in tp_repr to avoid accidental free#1160

lostmsu merged 1 commit intopythonnet:masterfromDanBarzilian:PR/fix-tp_repr-bug
Jun 17, 2020

Conversation

DanBarzilian
Copy link
Contributor

What does this implement/fix? Explain your changes.

Fix incorrect dereferencing wrapper object in tp_repr function, which causes the program to crash.

Does this close any currently open issues?

...

Any other comments?

...

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • [x ] Add yourself toAUTHORS
  • [x ] Updated theCHANGELOG

lostmsu reacted with rocket emoji
@dnfadmin
Copy link

dnfadmin commentedJun 17, 2020•
edited
Loading

CLA assistant check
All CLA requirements met.

@DanBarzilianDanBarzilian mentioned this pull requestJun 17, 2020
4 tasks
@codecov-commenter
Copy link

codecov-commenter commentedJun 17, 2020•
edited
Loading

Codecov Report

Merging#1160 intomaster willnot change coverage.
The diff coverage isn/a.

Impacted file tree graph

@@           Coverage Diff           @@##           master    #1160   +/-   ##=======================================  Coverage   86.53%   86.53%           =======================================  Files           1        1             Lines         297      297           =======================================  Hits          257      257             Misses         40       40
FlagCoverage Δ
#setup_linux64.98% <ø> (ø)
#setup_windows72.05% <ø> (ø)

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update9fb545a...89d1177. Read thecomment docs.

@filmorfilmor requested a review fromlostmsuJune 17, 2020 09:25
@filmor
Copy link
Member

@lostmsu We should probably do a point-release for this, agree?

lostmsu reacted with thumbs up emoji

@lostmsu
Copy link
Member

@filmor I have another one too in the recent params array change.

@lostmsulostmsu merged commit742463e intopythonnet:masterJun 17, 2020
@lostmsu
Copy link
Member

BTW,@DanBarzilian , a personal thank you 10x! This finding and the fix for it just saved me after 3 workdays of chasing a semi-random segfault in tests. I don't know how much time I would have wasted trying to find this.

Funnily, while searching for (apparently) this issue, I stumbled and fixed another less impacting one:losttech@74e7d62

DanBarzilian reacted with heart emoji

@lostmsu
Copy link
Member

@filmor@koubaa we should mandate using*Reference types in new PRs. This bug would have been prevented by proper typing.

filmor reacted with thumbs up emoji

@koubaa
Copy link
Contributor

@lostmsu now it explains why the CI has been unreliable recently. Big whoops for missing this and yes more evidence to use Reference types. Good catch@DanBarzilian

DanBarzilian reacted with thumbs up emoji

@DanBarzilianDanBarzilian deleted the PR/fix-tp_repr-bug branchJune 23, 2020 08:03
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@lostmsulostmsulostmsu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@DanBarzilian@dnfadmin@codecov-commenter@filmor@lostmsu@koubaa
[8]ページ先頭
©2009-2025 Movatter.jp