From 88e5ad1a07569447ff74968c2bcfd52582e5ffbf Mon Sep 17 00:00:00 2001 From: berryzplus Date: Sat, 12 Feb 2022 23:24:02 +0900 Subject: [PATCH 1/3] =?UTF-8?q?CFontAutoDeleter=E3=82=92=E3=83=AA=E3=83=95?= =?UTF-8?q?=E3=82=A1=E3=82=AF=E3=82=BF=E3=83=AA=E3=83=B3=E3=82=B0=E3=81=97?= =?UTF-8?q?=E3=81=A6=E5=AE=89=E5=85=A8=E6=80=A7=E3=82=92=E4=B8=8A=E3=81=92?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CFontAutoDeleterは似非スマートポインタ。 コピーコンストラクタが未実装で、削除済みフォントを何度も削除しようとしてWindows GDIにダメージを与える危険があったのを修正する。 --- sakura_core/util/window.cpp | 65 ++++++++++++------------- sakura_core/util/window.h | 24 ++++++---- tests/unittests/test-window.cpp | 66 ++++++++++++++++++++++++++ tests/unittests/tests1.vcxproj | 1 + tests/unittests/tests1.vcxproj.filters | 3 ++ 5 files changed, 115 insertions(+), 44 deletions(-) create mode 100644 tests/unittests/test-window.cpp diff --git a/sakura_core/util/window.cpp b/sakura_core/util/window.cpp index ac9dec87a2..8cad4618d3 100644 --- a/sakura_core/util/window.cpp +++ b/sakura_core/util/window.cpp @@ -287,56 +287,51 @@ int CTextWidthCalc::GetTextHeight() const return tm.tmHeight; } -CFontAutoDeleter::CFontAutoDeleter() - : m_hFontOld(NULL) - , m_hFont(NULL) - , m_hwnd(NULL) -{} +CFontAutoDeleter::CFontAutoDeleter(const Me& other) +{ + operator = (other); +} -CFontAutoDeleter::~CFontAutoDeleter() +CFontAutoDeleter& CFontAutoDeleter::operator = (const Me& other) { - if( m_hFont ){ - DeleteObject( m_hFont ); - m_hFont = NULL; + Clear(); + + if (const auto hFont = other.m_hFont) { + if (LOGFONT lf = {}; + ::GetObject(hFont, sizeof(lf), &lf)) { + m_hFont = ::CreateFontIndirect(&lf); + } } + + return *this; } -void CFontAutoDeleter::SetFont( HFONT hfontOld, HFONT hfont, HWND hwnd ) +CFontAutoDeleter::~CFontAutoDeleter() { - if( m_hFont ){ - ::DeleteObject( m_hFont ); - } - if( m_hFont != hfontOld ){ - m_hFontOld = hfontOld; - } - m_hFont = hfont; - m_hwnd = hwnd; + Clear(); } -/*! ウィンドウのリリース(WM_DESTROY用) -*/ -void CFontAutoDeleter::ReleaseOnDestroy() +void CFontAutoDeleter::Clear() { - if( m_hFont ){ - ::DeleteObject( m_hFont ); - m_hFont = NULL; + if (m_hFont) { + ::DeleteObject(m_hFont); + m_hFont = nullptr; } - m_hFontOld = NULL; } -/*! ウィンドウ生存中のリリース +void CFontAutoDeleter::SetFont( [[maybe_unused]] HFONT hFontOld, HFONT hFont, [[maybe_unused]] HWND hWnd ) +{ + Clear(); + + m_hFont = hFont; +} + +/*! ウィンドウのリリース(WM_DESTROY用) */ -#if 0 -void CFontAutoDeleter::Release() +void CFontAutoDeleter::ReleaseOnDestroy() { - if( m_hwnd && m_hFont ){ - ::SendMessageAny( m_hwnd, WM_SETFONT, (WPARAM)m_hFontOld, FALSE ); - ::DeleteObject( m_hFont ); - m_hFont = NULL; - m_hwnd = NULL; - } + Clear(); } -#endif /*! システムフォントに準拠したフォントを取得 diff --git a/sakura_core/util/window.h b/sakura_core/util/window.h index 181af5a245..5cd636f235 100644 --- a/sakura_core/util/window.h +++ b/sakura_core/util/window.h @@ -160,17 +160,23 @@ class CTextWidthCalc class CFontAutoDeleter { +private: + HFONT m_hFont = nullptr; + + using Me = CFontAutoDeleter; + + void Clear(); + public: - CFontAutoDeleter(); - ~CFontAutoDeleter(); - void SetFont( HFONT hfontOld, HFONT hfont, HWND hwnd ); - void ReleaseOnDestroy(); - // void Release(); + CFontAutoDeleter() = default; + CFontAutoDeleter(const Me& other); + Me& operator = (const Me& other); + virtual ~CFontAutoDeleter(); -private: - HFONT m_hFontOld; - HFONT m_hFont; - HWND m_hwnd; + void SetFont( HFONT hFontOld, HFONT hFont, HWND hWnd ); + void ReleaseOnDestroy(); + + [[nodiscard]] HFONT GetFont() const { return m_hFont; } }; class CDCFont diff --git a/tests/unittests/test-window.cpp b/tests/unittests/test-window.cpp new file mode 100644 index 0000000000..43f8b64e58 --- /dev/null +++ b/tests/unittests/test-window.cpp @@ -0,0 +1,66 @@ +/*! @file */ +/* + Copyright (C) 2022, Sakura Editor Organization + + This software is provided 'as-is', without any express or implied + warranty. In no event will the authors be held liable for any damages + arising from the use of this software. + + Permission is granted to anyone to use this software for any purpose, + including commercial applications, and to alter it and redistribute it + freely, subject to the following restrictions: + + 1. The origin of this software must not be misrepresented; + you must not claim that you wrote the original software. + If you use this software in a product, an acknowledgment + in the product documentation would be appreciated but is + not required. + + 2. Altered source versions must be plainly marked as such, + and must not be misrepresented as being the original software. + + 3. This notice may not be removed or altered from any source + distribution. +*/ +#include + +#ifndef NOMINMAX +#define NOMINMAX +#endif /* #ifndef NOMINMAX */ + +#include +#include +#include +#include + +#include + +#include "util/window.h" + +/*! + * @brief CFontAutoDeleterのテスト + */ +TEST( CFontAutoDeleter, test ) +{ + CFontAutoDeleter deletor; + ASSERT_EQ(nullptr, deletor.GetFont()); + + if (const auto hGdiFont = GetStockFont(DEFAULT_GUI_FONT)) { + if (LOGFONT lf = {}; + ::GetObject(hGdiFont, sizeof(lf), &lf)) { + if (const auto hFont = ::CreateFontIndirect(&lf)) { + deletor.SetFont(nullptr, hFont, nullptr); + ASSERT_EQ(hFont, deletor.GetFont()); + } + } + } + + ASSERT_NE(nullptr, deletor.GetFont()); + if (const auto hFont = deletor.GetFont()) { + CFontAutoDeleter other(deletor); + ASSERT_NE(hFont, other.GetFont()); + + other.ReleaseOnDestroy(); + ASSERT_EQ(nullptr, other.GetFont()); + } +} diff --git a/tests/unittests/tests1.vcxproj b/tests/unittests/tests1.vcxproj index 9c08a294fa..f45a2273a3 100644 --- a/tests/unittests/tests1.vcxproj +++ b/tests/unittests/tests1.vcxproj @@ -150,6 +150,7 @@ + diff --git a/tests/unittests/tests1.vcxproj.filters b/tests/unittests/tests1.vcxproj.filters index c5ec04926c..2d8cc79a47 100644 --- a/tests/unittests/tests1.vcxproj.filters +++ b/tests/unittests/tests1.vcxproj.filters @@ -157,6 +157,9 @@ Test Files + + Test Files + From 3c2922e7e61d0bea0e7da7299391ceb14edb03eb Mon Sep 17 00:00:00 2001 From: berryzplus Date: Sun, 13 Feb 2022 00:13:12 +0900 Subject: [PATCH 2/3] =?UTF-8?q?Code=20Smells=E5=AF=BE=E7=AD=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sakura_core/util/window.cpp | 19 +++++++++++++++++-- sakura_core/util/window.h | 6 ++++-- tests/unittests/test-window.cpp | 4 ++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/sakura_core/util/window.cpp b/sakura_core/util/window.cpp index 8cad4618d3..25452414a2 100644 --- a/sakura_core/util/window.cpp +++ b/sakura_core/util/window.cpp @@ -306,12 +306,27 @@ CFontAutoDeleter& CFontAutoDeleter::operator = (const Me& other) return *this; } +CFontAutoDeleter::CFontAutoDeleter(Me&& other) noexcept +{ + operator = (std::forward(other)); +} + +CFontAutoDeleter& CFontAutoDeleter::operator = (Me&& other) noexcept +{ + Clear(); + + m_hFont = other.m_hFont; + other.m_hFont = nullptr; + + return *this; +} + CFontAutoDeleter::~CFontAutoDeleter() { Clear(); } -void CFontAutoDeleter::Clear() +void CFontAutoDeleter::Clear() noexcept { if (m_hFont) { ::DeleteObject(m_hFont); @@ -319,7 +334,7 @@ void CFontAutoDeleter::Clear() } } -void CFontAutoDeleter::SetFont( [[maybe_unused]] HFONT hFontOld, HFONT hFont, [[maybe_unused]] HWND hWnd ) +void CFontAutoDeleter::SetFont( [[maybe_unused]] const HFONT hFontOld, const HFONT hFont, [[maybe_unused]] const HWND hWnd ) { Clear(); diff --git a/sakura_core/util/window.h b/sakura_core/util/window.h index 5cd636f235..7098870fa9 100644 --- a/sakura_core/util/window.h +++ b/sakura_core/util/window.h @@ -165,15 +165,17 @@ class CFontAutoDeleter using Me = CFontAutoDeleter; - void Clear(); + void Clear() noexcept; public: CFontAutoDeleter() = default; CFontAutoDeleter(const Me& other); Me& operator = (const Me& other); + CFontAutoDeleter(Me&& other) noexcept; + Me& operator = (Me&& other) noexcept; virtual ~CFontAutoDeleter(); - void SetFont( HFONT hFontOld, HFONT hFont, HWND hWnd ); + void SetFont( const HFONT hFontOld, const HFONT hFont, const HWND hWnd ); void ReleaseOnDestroy(); [[nodiscard]] HFONT GetFont() const { return m_hFont; } diff --git a/tests/unittests/test-window.cpp b/tests/unittests/test-window.cpp index 43f8b64e58..32dd0b6425 100644 --- a/tests/unittests/test-window.cpp +++ b/tests/unittests/test-window.cpp @@ -62,5 +62,9 @@ TEST( CFontAutoDeleter, test ) other.ReleaseOnDestroy(); ASSERT_EQ(nullptr, other.GetFont()); + + CFontAutoDeleter another = std::move(deletor); + ASSERT_EQ(hFont, another.GetFont()); + ASSERT_EQ(nullptr, deletor.GetFont()); } } From 3e6ec6954ced233aad977d0f21a71448a94ac426 Mon Sep 17 00:00:00 2001 From: berryzplus Date: Sun, 13 Feb 2022 02:12:37 +0900 Subject: [PATCH 3/3] =?UTF-8?q?=E5=AF=BE=E5=BF=9C=E5=8F=96=E6=B6=88?= =?UTF-8?q?=EF=BC=8B=E5=A4=89=E6=95=B0=E5=90=8D=E8=A8=82=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sakura_core/util/window.cpp | 6 +++--- sakura_core/util/window.h | 4 ++-- tests/unittests/test-window.cpp | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/sakura_core/util/window.cpp b/sakura_core/util/window.cpp index 25452414a2..e50dcccc33 100644 --- a/sakura_core/util/window.cpp +++ b/sakura_core/util/window.cpp @@ -308,7 +308,7 @@ CFontAutoDeleter& CFontAutoDeleter::operator = (const Me& other) CFontAutoDeleter::CFontAutoDeleter(Me&& other) noexcept { - operator = (std::forward(other)); + operator = (std::move(other)); } CFontAutoDeleter& CFontAutoDeleter::operator = (Me&& other) noexcept @@ -321,7 +321,7 @@ CFontAutoDeleter& CFontAutoDeleter::operator = (Me&& other) noexcept return *this; } -CFontAutoDeleter::~CFontAutoDeleter() +CFontAutoDeleter::~CFontAutoDeleter() noexcept { Clear(); } @@ -334,7 +334,7 @@ void CFontAutoDeleter::Clear() noexcept } } -void CFontAutoDeleter::SetFont( [[maybe_unused]] const HFONT hFontOld, const HFONT hFont, [[maybe_unused]] const HWND hWnd ) +void CFontAutoDeleter::SetFont( [[maybe_unused]] const HFONT& hFontOld, const HFONT& hFont, [[maybe_unused]] const HWND& hWnd ) { Clear(); diff --git a/sakura_core/util/window.h b/sakura_core/util/window.h index 7098870fa9..146f79cba7 100644 --- a/sakura_core/util/window.h +++ b/sakura_core/util/window.h @@ -173,9 +173,9 @@ class CFontAutoDeleter Me& operator = (const Me& other); CFontAutoDeleter(Me&& other) noexcept; Me& operator = (Me&& other) noexcept; - virtual ~CFontAutoDeleter(); + virtual ~CFontAutoDeleter() noexcept; - void SetFont( const HFONT hFontOld, const HFONT hFont, const HWND hWnd ); + void SetFont( const HFONT& hFontOld, const HFONT& hFont, const HWND& hWnd ); void ReleaseOnDestroy(); [[nodiscard]] HFONT GetFont() const { return m_hFont; } diff --git a/tests/unittests/test-window.cpp b/tests/unittests/test-window.cpp index 32dd0b6425..5a21a232f6 100644 --- a/tests/unittests/test-window.cpp +++ b/tests/unittests/test-window.cpp @@ -42,29 +42,29 @@ */ TEST( CFontAutoDeleter, test ) { - CFontAutoDeleter deletor; - ASSERT_EQ(nullptr, deletor.GetFont()); + CFontAutoDeleter deleter; + ASSERT_EQ(nullptr, deleter.GetFont()); if (const auto hGdiFont = GetStockFont(DEFAULT_GUI_FONT)) { if (LOGFONT lf = {}; ::GetObject(hGdiFont, sizeof(lf), &lf)) { if (const auto hFont = ::CreateFontIndirect(&lf)) { - deletor.SetFont(nullptr, hFont, nullptr); - ASSERT_EQ(hFont, deletor.GetFont()); + deleter.SetFont(nullptr, hFont, nullptr); + ASSERT_EQ(hFont, deleter.GetFont()); } } } - ASSERT_NE(nullptr, deletor.GetFont()); - if (const auto hFont = deletor.GetFont()) { - CFontAutoDeleter other(deletor); + ASSERT_NE(nullptr, deleter.GetFont()); + if (const auto hFont = deleter.GetFont()) { + CFontAutoDeleter other(deleter); ASSERT_NE(hFont, other.GetFont()); other.ReleaseOnDestroy(); ASSERT_EQ(nullptr, other.GetFont()); - CFontAutoDeleter another = std::move(deletor); + CFontAutoDeleter another(std::move(deleter)); ASSERT_EQ(hFont, another.GetFont()); - ASSERT_EQ(nullptr, deletor.GetFont()); + ASSERT_EQ(nullptr, deleter.GetFont()); } }