search

AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.47k stars 294 forks source link

#1680 fix segmentation fault when appending None #1778

Closed semagnum closed 4 months ago

semagnum commented 4 months ago

Fixes #1680

Summary

Changes to reference parameter to raise TypeError instead of crashing with a segmentation fault. Created test_append_none to prevent regression.

linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 81.70%. Comparing base (c0e97b0) to head (6114856). Report is 18 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778/graphs/tree.svg?width=650&height=150&src=pr&token=mfnfUB4h7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation)](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ```diff @@ Coverage Diff @@ ## main #1778 +/- ## ========================================== - Coverage 84.11% 81.70% -2.41% ========================================== Files 198 176 -22 Lines 22241 12318 -9923 Branches 4687 3028 -1659 ========================================== - Hits 18709 10065 -8644 + Misses 2610 1717 -893 + Partials 922 536 -386 ``` | [Flag](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [py-unittests](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | `81.70% <80.00%> (-2.41%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [...entimelineio-bindings/otio\_serializableObjects.cpp](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?src=pr&el=tree&filepath=src%2Fpy-opentimelineio%2Fopentimelineio-bindings%2Fotio_serializableObjects.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL3B5LW9wZW50aW1lbGluZWlvL29wZW50aW1lbGluZWlvLWJpbmRpbmdzL290aW9fc2VyaWFsaXphYmxlT2JqZWN0cy5jcHA=) | `93.66% <100.00%> (-0.20%)` | :arrow_down: | | [tests/test\_track.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?src=pr&el=tree&filepath=tests%2Ftest_track.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF90cmFjay5weQ==) | `93.47% <66.66%> (-1.88%)` | :arrow_down: | ... and [65 files with indirect coverage changes](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Last update [e10fdd2...6114856](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1778?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation).
ssteinbach commented 4 months ago

Just to make sure I understand- because it was pointer before, you could pass the python None value, which would coerce to a nullptr and then cause a segfault when adding. By switching to a ref, pybind disallows nullptr/None and raises a TypeError?