From 52826cc41a12430f073c433c74a9a0b1e153f2d3 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 20 May 2025 11:46:29 +0200 Subject: [PATCH] QPainter: make ownership transfer more explicit The push_back(get()) / Q_UNUSED(release()) was an attempt to separate the reallocation from the release of the unique_ptr, to avoid memory leaks in case the reallocation fails inside push_back(), at a time when unique_ptr::release() had already been called. Coverity doesn't understand this pattern (and many human readers won't, either), so change this into an emplace_back(), followed by a release() into the return value of emplace_back(). Since assignments evaluate right to left, we need to separate these two actions into separate statements, otherwise we'd end up in the same situation as described for push_back(release()) above. Add a comment explaining the problem. Amends the start of the public history. Coverity-Id: 425479 Pick-to: 6.8 6.5 Change-Id: I727aaf791d7b523cd6e7301c49c152b4b6e80dd2 Reviewed-by: Axel Spoerl (cherry picked from commit 603499d2e7cd2830f0d3ff7b97f1c2fb430f2118) Reviewed-by: Qt Cherry-pick Bot --- src/gui/painting/qpainter.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp index f5285a9120f..2982f05f2a3 100644 --- a/src/gui/painting/qpainter.cpp +++ b/src/gui/painting/qpainter.cpp @@ -247,8 +247,11 @@ bool QPainterPrivate::attachPainterPrivate(QPainter *q, QPaintDevice *pdev) // the current d_ptr to the shared painter's d_ptr. sp->save(); ++sp->d_ptr->refcount; - sp->d_ptr->d_ptrs.push_back(q->d_ptr.get()); - Q_UNUSED(q->d_ptr.release()); + { + // ensure realloc happens before the unique_ptr::release(): + auto &p = sp->d_ptr->d_ptrs.emplace_back(); + p = q->d_ptr.release(); + } q->d_ptr.reset(sp->d_ptr.get()); Q_ASSERT(q->d_ptr->state);