Differential D28889 Diff 80409 plugins/platforms/drm/drm_output.cpp

Changeset View

Standalone View

View Options

plugins/platforms/drm/drm_output.cpp

Show First 20 Lines • Show All 59 Lines • ▼ Show 20 Line(s)
60{ 60​{
61 Q_ASSERT(!m_pageFlipPending); 61​ Q_ASSERT(!m_pageFlipPending);
62 teardown(); 62​ teardown();
63} 63​}
64 64​
65void DrmOutput::teardown() 65​void DrmOutput::teardown()
66{ 66​{
67 if (m_deleted) { 67​ if (m_deleted) {
68 return; 68​ return;
bportUnsubmitted
Done

no space between return and ;

bport: no space between return and ;
69 } 69​ }
70 m_deleted = true; 70​ m_deleted = true;
71 hideCursor(); 71​ hideCursor();
72 m_crtc->blank(); 72​ m_crtc->blank();
73 73​
74 if (m_primaryPlane) { 74​ if (m_primaryPlane) {
75 // TODO: when having multiple planes, also clean up these 75​ // TODO: when having multiple planes, also clean up these
76 m_primaryPlane->setOutput(nullptr); 76​ m_primaryPlane->setOutput(nullptr);
77 77​
78 if (m_backend->deleteBufferAfterPageFlip()) { 78​ if (m_backend->deleteBufferAfterPageFlip()) {
79 delete m_primaryPlane->current(); 79​ delete m_primaryPlane->current();
80 } 80​ }
81 m_primaryPlane->setCurrent(nullptr); 81​ m_primaryPlane->setCurrent(nullptr);
82 } 82​ }
83​ if (m_cursorPlane) {
84​ m_cursorPlane->setOutput(nullptr);
85​ }
83 86​
84 m_crtc->setOutput(nullptr); 87​ m_crtc->setOutput(nullptr);
85 m_conn->setOutput(nullptr); 88​ m_conn->setOutput(nullptr);
davidedmundsonUnsubmitted
Done

From what I can tell we never call m_cursorPlane->setCurrent with anything non-null? So this is all a no-op.

The buffers are the m_cursor buffers deleted below, and they're set directly below.

davidedmundson: From what I can tell we never call m_cursorPlane->setCurrent with anything non-null? So this is…
86 89​
87 delete m_cursor[0]; 90​ delete m_cursor[0];
91​ m_cursor[0] = nullptr;
88 delete m_cursor[1]; 92​ delete m_cursor[1];
93​ m_cursor[1] = nullptr;
mevenAuthorUnsubmitted
Done

I replace those deletes, as they did not delete the m_cursor array and allowed m_cursor[i] to keep dangling pointers causing down the line crash in QImage rather than a proper segfault where m_cursor[i] was used.

meven: I replace those deletes, as they did not delete the m_cursor array and allowed m_cursor[i] to…
zzagUnsubmitted
Done

m_cursor is not a dynamically allocated array. We should not delete[] it.

zzag: m_cursor is not a dynamically allocated array. We should not `delete[]` it.
fvogtUnsubmitted
Done

To avoid dangling pointers it would have to use

delete m_cursor[0];
m_cursor[0] = nullptr;

delete[] m_cursor is like free(m_cursor). It'll most likely crash.

fvogt: To avoid dangling pointers it would have to use ``` delete m_cursor[0]; m_cursor[0] = nullptr…
89 if (!m_pageFlipPending) { 94​ if (!m_pageFlipPending) {
90 deleteLater(); 95​ deleteLater();
91 } //else will be deleted in the page flip handler 96​ } //else will be deleted in the page flip handler
92 //this is needed so that the pageflipcallback handle isn't deleted 97​ //this is needed so that the pageflipcallback handle isn't deleted
93} 98​}
94 99​
95void DrmOutput::releaseGbm() 100​void DrmOutput::releaseGbm()
96{ 101​{
▲ Show 20 Lines • Show All 64 Lines • ▼ Show 20 Line(s)161 if (angle) {
161 matrix.translate(-center.width(), -center.height()); 166​ matrix.translate(-center.width(), -center.height());
162 } 167​ }
163 matrix.scale(scale()); 168​ matrix.scale(scale());
164 return matrix; 169​ return matrix;
165} 170​}
166 171​
167void DrmOutput::updateCursor() 172​void DrmOutput::updateCursor()
168{ 173​{
174​ if (m_deleted) {
175​ return;
apolUnsubmitted
Done

Shouldn't have the space return;

apol: Shouldn't have the space `return;`
bportUnsubmitted
Done

you still have the space :-)

bport: you still have the space :-)
mevenAuthorUnsubmitted
Done

@apol implied they were needed... And the file has both style mixed.

meven: @apol implied they were needed... And the file has both style mixed.
bportUnsubmitted
Not Done

I thought he wanted you to remove it

Shouldn't have the space return;

bport: I thought he wanted you to remove it > Shouldn't have the space return;
176​ }
169 QImage cursorImage = m_backend->softwareCursor(); 177​ QImage cursorImage = m_backend->softwareCursor();
170 if (cursorImage.isNull()) { 178​ if (cursorImage.isNull()) {
171 return; 179​ return;
172 } 180​ }
173 m_hasNewCursor = true; 181​ m_hasNewCursor = true;
174 QImage *c = m_cursor[m_cursorIndex]->image(); 182​ QImage *c = m_cursor[m_cursorIndex]->image();
175 c->fill(Qt::transparent); 183​ c->fill(Qt::transparent);
176 184​
▲ Show 20 Lines • Show All 930 Lines • Show Last 20 Lines