Remove FPDFPageObj_Destroy() calls in fpdf_edit_embeddertest.cpp [pdfium : main]
Andy Phan (Gerrit)
Andy Phan added 2 comments![]( "Open in Gerrit")
ScopedFPDFPageObject page_object_deleter(page_object);On a similar note, should this line occur after the FPDFPage_RemoveObject call? And should it be changed to ASSERT_TRUE?
// Not calling release() since ownership will be returned below in theIs this a little more dangerous? Could there be a double free possible on test failure?
Related details
- Lei Zhang
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Lei Zhang added 2 comments![]( "Open in Gerrit")
On a similar note, should this line occur after the FPDFPage_RemoveObject call? And should it be changed to ASSERT_TRUE?
The only ways for FPDFPage_RemoveObject() to fail here are:
1) `page` is invalid, but that shouldn't happen and the test would probably have failed or crashed before getting here.
2) `page_object` is null. In which case, there's no chance of a double free.
// Not calling release() since ownership will be returned below in theIs this a little more dangerous? Could there be a double free possible on test failure?
There's no ASSERT_FOO() call between here and line 2243, but I can release() here.
Related details
- Andy Phan
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Phan (Gerrit)
Andy Phan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andy Phan
| Code-Review | +1 |
1 comment
ScopedFPDFPageObject page_object_deleter(page_object);Lei ZhangOn a similar note, should this line occur after the FPDFPage_RemoveObject call? And should it be changed to ASSERT_TRUE?
The only ways for FPDFPage_RemoveObject() to fail here are:
1) `page` is invalid, but that shouldn't happen and the test would probably have failed or crashed before getting here.
2) `page_object` is null. In which case, there's no chance of a double free.
- Lei Zhang
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Pdfium LUCI CQ (Gerrit)
Pdfium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Remove FPDFPageObj_Destroy() calls in fpdf_edit_embeddertest.cpp
Use ScopedFPDFPageObject instead.
- M fpdfsdk/fpdf_edit_embeddertest.cpp
Code-Review: +1 by Andy Phan
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |