Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SonarQubeの静的解析で検出されたCFontAutoDeleterのバグを修正する #1799

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 45 additions & 35 deletions sakura_core/util/window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,56 +287,66 @@ 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(Me&& other) noexcept
{
if( m_hFont ){
::DeleteObject( m_hFont );
}
if( m_hFont != hfontOld ){
m_hFontOld = hfontOld;
}
m_hFont = hfont;
m_hwnd = hwnd;
operator = (std::move(other));
}

/*! ウィンドウのリリース(WM_DESTROY用)
*/
void CFontAutoDeleter::ReleaseOnDestroy()
CFontAutoDeleter& CFontAutoDeleter::operator = (Me&& other) noexcept
{
if( m_hFont ){
::DeleteObject( m_hFont );
m_hFont = NULL;
Clear();

m_hFont = other.m_hFont;
other.m_hFont = nullptr;

return *this;
}

CFontAutoDeleter::~CFontAutoDeleter() noexcept
{
Clear();
}

void CFontAutoDeleter::Clear() noexcept
{
if (m_hFont) {
::DeleteObject(m_hFont);
m_hFont = nullptr;
}
m_hFontOld = NULL;
}

/*! ウィンドウ生存中のリリース
void CFontAutoDeleter::SetFont( [[maybe_unused]] const HFONT& hFontOld, const HFONT& hFont, [[maybe_unused]] const 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

/*!
システムフォントに準拠したフォントを取得
Expand Down
26 changes: 17 additions & 9 deletions sakura_core/util/window.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,25 @@ class CTextWidthCalc

class CFontAutoDeleter
{
private:
HFONT m_hFont = nullptr;

using Me = CFontAutoDeleter;

void Clear() noexcept;

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);
CFontAutoDeleter(Me&& other) noexcept;
Me& operator = (Me&& other) noexcept;
virtual ~CFontAutoDeleter() noexcept;

private:
HFONT m_hFontOld;
HFONT m_hFont;
HWND m_hwnd;
void SetFont( const HFONT& hFontOld, const HFONT& hFont, const HWND& hWnd );
void ReleaseOnDestroy();

[[nodiscard]] HFONT GetFont() const { return m_hFont; }
};

class CDCFont
Expand Down
70 changes: 70 additions & 0 deletions tests/unittests/test-window.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*! @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 <gtest/gtest.h>

#ifndef NOMINMAX
#define NOMINMAX
#endif /* #ifndef NOMINMAX */

#include <tchar.h>
#include <Windows.h>
#include <windowsx.h>
#include <Shlwapi.h>

#include <cstdlib>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要なヘッダをインクルードしているように見えます。

  • tchar.h
  • shlwapi.h
  • cstdlib

細かい点ですので修正してほしいという要求ではありません。


#include "util/window.h"

/*!
* @brief CFontAutoDeleterのテスト
*/
TEST( CFontAutoDeleter, test )
{
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)) {
deleter.SetFont(nullptr, hFont, nullptr);
ASSERT_EQ(hFont, deleter.GetFont());
}
}
}

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(deleter));
ASSERT_EQ(hFont, another.GetFont());
ASSERT_EQ(nullptr, deleter.GetFont());
}
}
1 change: 1 addition & 0 deletions tests/unittests/tests1.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@
<ClCompile Include="test-statictype.cpp" />
<ClCompile Include="test-StdControl.cpp" />
<ClCompile Include="test-string_ex.cpp" />
<ClCompile Include="test-window.cpp" />
<ClCompile Include="test-zoom.cpp" />
<ClCompile Include="test-winmain.cpp" />
</ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions tests/unittests/tests1.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@
<ClCompile Include="test-statictype.cpp">
<Filter>Test Files</Filter>
</ClCompile>
<ClCompile Include="test-window.cpp">
<Filter>Test Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="StartEditorProcessForTest.h">
Expand Down