Changeset View
Changeset View
Standalone View
Standalone View
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 | | |||
| 65 | void DrmOutput::teardown() | 65 | void DrmOutput::teardown() | ||
| 66 | { | 66 | { | ||
| 67 | if (m_deleted) { | 67 | if (m_deleted) { | ||
| 68 | return; | 68 | return; | ||
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); | ||
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; | ||||
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… | |||||
zzag: m_cursor is not a dynamically allocated array. We should not `delete[]` it. | |||||
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 | | |||
| 95 | void 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 | | |||
| 167 | void DrmOutput::updateCursor() | 172 | void DrmOutput::updateCursor() | ||
| 168 | { | 173 | { | ||
| 174 | if (m_deleted) { | ||||
| 175 | return; | ||||
apol: Shouldn't have the space `return;` | |||||
bport: you still have the space :-) | |||||
@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. | |||||
bport: I thought he wanted you to remove it
> Shouldn't have the space return;
| |||||
| 176 | } | ||||
| 169 | QImage cursorImage = Cursors::self()->currentCursor()->image(); | 177 | QImage cursorImage = Cursors::self()->currentCursor()->image(); | ||
| 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 922 Lines • Show Last 20 Lines | |||||
no space between return and ;