From 2255d843969536901823b98ff8ca9dfb9450b05b Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Mon, 25 Mar 2024 18:26:16 +0100
Subject: [PATCH] QMainWindowLayout: rewrite validateToolBarArea() to return by
 value

Coverity complains that QToolBarAreaLayout's
addToolBarBreak(QInternal::DockPosition) could access
QToolBarAreaLayout::docks out of bounds if passed
QInternal::DockCount.

That is correct, but a valid pos seems to be a precondition for this
function, judging from its sister functions, e.g.
addToolBar(DockPosition, .) or insertItem(DockPosition, .), which also
don't validate `pos`. All in-module callers of addToolBarBreak() only
pass valid positions, and use validateToolBarArea() to ensure that. So
it seems that Coverity doesn't grok the pass-by-in/out -parameter used
by that function. That, or it doesn't track back far enough.

Before attempting more drastic measures, first try rewriting the
function to return-by-value instead, and see what Coverity has to say
afterwards.

As a drive-by, make validateToolBarArea() constexpr.

Pick-to: 6.5 6.2 5.15
Coverity-Id: 444141
Coverity-Id: 444135
Change-Id: I5fcc664c3cea608429036cad75c37f5c38059733
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 19aeb431cf1bd4e864356ff02db6337dc59b2835)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 59072e9916278db1fc104656ba9fbf3e65317367)
---
 src/widgets/widgets/qmainwindowlayout.cpp | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp
index e17fa28b832..55781a567f0 100644
--- a/src/widgets/widgets/qmainwindowlayout.cpp
+++ b/src/widgets/widgets/qmainwindowlayout.cpp
@@ -1430,17 +1430,18 @@ bool QMainWindowLayoutState::restoreState(QDataStream &_stream,
 
 #if QT_CONFIG(toolbar)
 
-static inline void validateToolBarArea(Qt::ToolBarArea &area)
+static constexpr Qt::ToolBarArea validateToolBarArea(Qt::ToolBarArea area)
 {
     switch (area) {
     case Qt::LeftToolBarArea:
     case Qt::RightToolBarArea:
     case Qt::TopToolBarArea:
     case Qt::BottomToolBarArea:
-        break;
+        return area;
     default:
-        area = Qt::TopToolBarArea;
+        break;
     }
+    return Qt::TopToolBarArea;
 }
 
 static QInternal::DockPosition toDockPos(Qt::ToolBarArea area)
@@ -1476,7 +1477,7 @@ static inline Qt::ToolBarArea toToolBarArea(int pos)
 
 void QMainWindowLayout::addToolBarBreak(Qt::ToolBarArea area)
 {
-    validateToolBarArea(area);
+    area = validateToolBarArea(area);
 
     layoutState.toolBarAreaLayout.addToolBarBreak(toDockPos(area));
     if (savedState.isValid())
@@ -1530,7 +1531,7 @@ void QMainWindowLayout::addToolBar(Qt::ToolBarArea area,
                                    QToolBar *toolbar,
                                    bool)
 {
-    validateToolBarArea(area);
+    area = validateToolBarArea(area);
     // let's add the toolbar to the layout
     addChildWidget(toolbar);
     QLayoutItem *item = layoutState.toolBarAreaLayout.addToolBar(toDockPos(area), toolbar);