Skip to content

Commit

Permalink
removed critical section global/static variable
Browse files Browse the repository at this point in the history
  • Loading branch information
@ariccio
ariccio committed Feb 10, 2015
1 parent 8fd0ab6 commit 113b38b
Show file tree
Hide file tree
Showing 7 changed files with 301 additions and 240 deletions.
393 changes: 213 additions & 180 deletions WinDirStat/windirstat/SelectDrivesDlg.cpp
View file

Large diffs are not rendered by default.

34 changes: 29 additions & 5 deletions WinDirStat/windirstat/SelectDrivesDlg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,14 @@ class CDriveItem final : public COwnerDrawnListItem {
private:
//concrete_compare is called as a single line INSIDE a single line function. Let's ask for inlining.
inline INT concrete_compare ( _In_ const CDriveItem* const other, RANGE_ENUM_COL const column::ENUM_COL subitem ) const;
virtual INT Compare ( _In_ const COwnerDrawnListItem* const other, RANGE_ENUM_COL const column::ENUM_COL subitem ) const override final;

virtual INT Compare( _In_ const COwnerDrawnListItem* const baseOther, RANGE_ENUM_COL const column::ENUM_COL subitem ) const override final {
if ( subitem == column::COL_NAME ) {
return default_compare( baseOther );
}
const auto other = static_cast<const CDriveItem*>( baseOther );
return concrete_compare( other, subitem );
}


//CDriveItem NEVER draws self.
Expand All @@ -54,7 +61,22 @@ class CDriveItem final : public COwnerDrawnListItem {
}

_Must_inspect_result_ _Success_( SUCCEEDED( return ) )
virtual HRESULT Text_WriteToStackBuffer( RANGE_ENUM_COL const column::ENUM_COL subitem, WDS_WRITES_TO_STACK( strSize, chars_written ) PWSTR psz_text, _In_ const rsize_t strSize, _On_failure_( _Post_valid_ ) rsize_t& sizeBuffNeed, _Out_ rsize_t& chars_written ) const override final;
virtual HRESULT Text_WriteToStackBuffer( RANGE_ENUM_COL const column::ENUM_COL subitem, WDS_WRITES_TO_STACK( strSize, chars_written ) PWSTR psz_text, _In_ const rsize_t strSize, _On_failure_( _Post_valid_ ) rsize_t& sizeBuffNeed, _Out_ rsize_t& chars_written ) const override final {
switch ( subitem )
{
case column::COL_TOTAL:
case column::COL_FREE:
//return Text_WriteToStackBuffer_COL_TOTAL( subitem, psz_text, strSize, sizeBuffNeed, chars_written );
return wds_fmt::FormatBytes( ( ( subitem == column::COL_TOTAL ) ? m_totalBytes : m_freeBytes ), psz_text, strSize, chars_written, sizeBuffNeed );
case column::COL_NAME:
case column::COL_ITEMS:
case column::COL_BYTESPERCENT:
case column::COL_FILES_TYPEVIEW:
case column::COL_ATTRIBUTES:
default:
return WriteToStackBuffer_default( subitem, psz_text, strSize, sizeBuffNeed, chars_written, L"CDriveItem::" );
}
}

public:
const std::wstring m_path; // e.g. "C:\"
Expand All @@ -73,7 +95,7 @@ class CDriveInformationThread final : public CWinThread {
CDriveInformationThread& operator=( const CDriveInformationThread& in ) = delete;
CDriveInformationThread( const CDriveInformationThread& in ) = delete;

CDriveInformationThread( _In_ std::wstring path, LPARAM driveItem, HWND dialog, UINT serial, rsize_t thread_num );
CDriveInformationThread( _In_ std::wstring path, LPARAM driveItem, HWND dialog, UINT serial, rsize_t thread_num, _In_ CRITICAL_SECTION* const cs_in, _In_ std::vector<CDriveInformationThread*>* const dlg_in );
virtual ~CDriveInformationThread( ) final = default;
virtual BOOL InitInstance ( ) override final;

Expand All @@ -92,7 +114,8 @@ class CDriveInformationThread final : public CWinThread {
std::atomic<std::uint64_t> m_freeBytes; // Result: free space on the drive, valid if m_success
//C4820: 'CDriveInformationThread' : '7' bytes padding added after data member 'CDriveInformationThread::m_success'
std::atomic<bool> m_success; // Result: false, iff drive is unaccessible.

CRITICAL_SECTION* dialog_CRITICAL_SECTION_running_threads;
_Guarded_by_( dialog_CRITICAL_SECTION_running_threads ) std::vector<CDriveInformationThread*>* dialog_running_threads;
};

class CDrivesList final : public COwnerDrawnListCtrl {
Expand Down Expand Up @@ -159,7 +182,8 @@ class CSelectDrivesDlg final : public CDialog {
int m_radio; // out.
std::wstring m_folder_name_heap;// out. Valid if m_radio = RADIO_AFOLDER
std::vector<std::wstring> m_drives; // out. Valid if m_radio != RADIO_AFOLDER

CRITICAL_SECTION m_running_threads_CRITICAL_SECTION;
_Guarded_by_( m_running_threads_CRITICAL_SECTION ) std::vector<CDriveInformationThread*> m_running_threads;
protected:
static UINT _serial; // Each Instance of this dialog gets a serial number
CDrivesList m_list;
Expand Down
7 changes: 0 additions & 7 deletions WinDirStat/windirstat/datastructures.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,13 +369,6 @@ void copy_attribs( _Out_ attribs& out, _In_ const attribs& in ) {
}


struct children_string_heap_manager {
children_string_heap_manager( const rsize_t number_of_characters_needed ) m_buffer_size( number_of_characters_needed ), m_buffer_filled( 0 ), m_string_buffer( new wchar_t[ number_of_characters_needed ] ) { }

_Field_size_part_( m_buffer_size, m_buffer_filled ) std::unique_ptr<wchar_t[ ]> m_string_buffer;
const size_t m_buffer_size;
size_t m_buffer_filled;
};


namespace UpdateAllViews_ENUM {
Expand Down
40 changes: 40 additions & 0 deletions WinDirStat/windirstat/globalhelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,38 @@ namespace {

}

Children_String_Heap_Manager::Children_String_Heap_Manager( const rsize_t number_of_characters_needed ) : m_buffer_size( number_of_characters_needed ), m_buffer_filled( 0 ), m_string_buffer( new wchar_t[ number_of_characters_needed ] ) { }

_Success_( SUCCEEDED( return ) )
const HRESULT Children_String_Heap_Manager::copy_name_str_into_buffer( _Pre_invalid_ _Post_z_ _Post_readable_size_( new_name_length ) wchar_t*& new_name_ptr, _In_ _In_range_( 0, UINT16_MAX ) const rsize_t& new_name_length, const std::wstring& name ) {
ASSERT( new_name_length < UINT16_MAX );
new_name_ptr = ( m_string_buffer.get( ) + m_buffer_filled );
ASSERT( ( m_buffer_filled + new_name_length ) < m_buffer_size );
m_buffer_filled += new_name_length;

PWSTR pszend = NULL;

//god this is ugly.
const rsize_t buffer_space_remaining = ( m_buffer_size - m_buffer_filled + new_name_length );

rsize_t chars_remaining = buffer_space_remaining;
const HRESULT res = StringCchCopyExW( new_name_ptr, ( buffer_space_remaining ), name.c_str( ), &pszend, &chars_remaining, 0 );
ASSERT( SUCCEEDED( res ) );
if ( SUCCEEDED( res ) ) {
#ifdef DEBUG
ASSERT( wcslen( new_name_ptr ) == new_name_length );
ASSERT( wcscmp( new_name_ptr, name.c_str( ) ) == 0 );
const auto da_ptrdiff = ( std::ptrdiff_t( pszend ) - std::ptrdiff_t( new_name_ptr ) );
ASSERT( ( da_ptrdiff / sizeof( wchar_t ) ) == new_name_length );
#endif
return res;
}
displayWindowsMsgBoxWithMessage( L"Copy of name_str into Children_String_Heap_Manager failed!!!" );
std::terminate( );
return res;
}



QPC_timer::QPC_timer( ) : m_frequency( help_QueryPerformanceFrequency( ).QuadPart ), m_start( 0 ), m_end( 0 ) { }

Expand All @@ -470,6 +502,14 @@ void QPC_timer::end( ) {
m_end = help_QueryPerformanceCounter( ).QuadPart;
}

void InitializeCriticalSection_wrapper( _Pre_invalid_ _Post_valid_ _Out_ CRITICAL_SECTION& cs ) {
InitializeCriticalSection( &cs );
}

void DeleteCriticalSection_wrapper( _Pre_valid_ _Post_invalid_ CRITICAL_SECTION& cs ) {
DeleteCriticalSection( &cs );
}


void error_getting_pointer_to( _In_z_ PCWSTR const function_name ) {
std::wstring message;
Expand Down
40 changes: 9 additions & 31 deletions WinDirStat/windirstat/globalhelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,42 +37,19 @@ inline type_struct_to_init zero_init_struct( ) {


struct Children_String_Heap_Manager {

//TODO: I'm not using these yet, but if I define them inline, the compiler bitches that they're not used.
Children_String_Heap_Manager( const rsize_t number_of_characters_needed );
Children_String_Heap_Manager& operator=( const Children_String_Heap_Manager& in ) = delete;
Children_String_Heap_Manager( const rsize_t number_of_characters_needed ) : m_buffer_size( number_of_characters_needed ), m_buffer_filled( 0 ), m_string_buffer( new wchar_t[ number_of_characters_needed ] ) { }
~Children_String_Heap_Manager( ) = default;
Children_String_Heap_Manager( const Children_String_Heap_Manager& in ) = delete;

_Success_( SUCCEEDED( return ) )
const HRESULT copy_name_str_into_buffer( _Pre_invalid_ _Post_z_ _Post_readable_size_( new_name_length ) wchar_t*& new_name_ptr, _In_ _In_range_( 0, UINT16_MAX ) const rsize_t& new_name_length, const std::wstring& name );

_Field_size_part_( m_buffer_size, m_buffer_filled ) std::unique_ptr<wchar_t[ ]> m_string_buffer;
const size_t m_buffer_size;
size_t m_buffer_filled;

_Success_( SUCCEEDED( return ) )
const HRESULT copy_name_str_into_buffer( _Pre_invalid_ _Post_z_ _Post_readable_size_( new_name_length ) wchar_t*& new_name_ptr, _In_ _In_range_( 0, UINT16_MAX ) const rsize_t& new_name_length, const std::wstring& name ) {
ASSERT( new_name_length < UINT16_MAX );
new_name_ptr = ( m_string_buffer.get( ) + m_buffer_filled );
ASSERT( ( m_buffer_filled + new_name_length ) < m_buffer_size );
m_buffer_filled += new_name_length;

PWSTR pszend = NULL;

//god this is ugly.
const rsize_t buffer_space_remaining = ( m_buffer_size - m_buffer_filled + new_name_length );

rsize_t chars_remaining = buffer_space_remaining;
const HRESULT res = StringCchCopyExW( new_name_ptr, ( buffer_space_remaining ), name.c_str( ), &pszend, &chars_remaining, 0 );
ASSERT( SUCCEEDED( res ) );
if ( SUCCEEDED( res ) ) {
#ifdef DEBUG
ASSERT( wcslen( new_name_ptr ) == new_name_length );
ASSERT( wcscmp( new_name_ptr, name.c_str( ) ) == 0 );
const auto da_ptrdiff = ( std::ptrdiff_t( pszend ) - std::ptrdiff_t( new_name_ptr ) );
ASSERT( ( da_ptrdiff / sizeof( wchar_t ) ) == new_name_length );
#endif
return res;
}
displayWindowsMsgBoxWithMessage( L"Copy of name_str into Children_String_Heap_Manager failed!!!" );
std::terminate( );
return res;
}
};


Expand Down Expand Up @@ -102,8 +79,9 @@ void normalize_RECT( _Inout_ RECT& rect );
void error_getting_pointer_to( _In_z_ PCWSTR const function_name );
void test_if_null_funcptr( void* func_ptr, _In_z_ PCWSTR const function_name );

void InitializeCriticalSection_wrapper( _Pre_invalid_ _Post_valid_ _Out_ CRITICAL_SECTION& cs );


void DeleteCriticalSection_wrapper( _Pre_valid_ _Post_invalid_ CRITICAL_SECTION& cs );

//On returning E_FAIL, call GetLastError for details. That's not my idea!
_Success_( SUCCEEDED( return ) ) HRESULT CStyle_GetLastErrorAsFormattedMessage( WDS_WRITES_TO_STACK( strSize, chars_written ) PWSTR psz_formatted_error, _In_range_( 128, 32767 ) const rsize_t strSize, _Out_ rsize_t& chars_written, const DWORD error = GetLastError( ) );
Expand Down
17 changes: 0 additions & 17 deletions WinDirStat/windirstat/ownerdrawnlistcontrol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,23 +639,6 @@ class COwnerDrawnListCtrl : public CListCtrl {
ASSERT( SUCCEEDED( fmt_res ) );
if ( !SUCCEEDED( fmt_res ) ) {
handle_formatting_error_COwnerDrawnListCtrl_SortItems( fmt_res );

//displayWindowsMsgBoxWithMessage( L"Error in COwnerDrawnListCtrl::SortItems - StringCchPrintfW failed!(aborting)" );
//if ( fmt_res == STRSAFE_E_END_OF_FILE ) {
// displayWindowsMsgBoxWithMessage( L"Error in COwnerDrawnListCtrl::SortItems - StringCchPrintfW failed, STRSAFE_E_END_OF_FILE!(aborting)" );
// std::terminate( );
// }
//if ( fmt_res == STRSAFE_E_INSUFFICIENT_BUFFER ) {
// displayWindowsMsgBoxWithMessage( L"Error in COwnerDrawnListCtrl::SortItems - StringCchPrintfW failed, STRSAFE_E_INSUFFICIENT_BUFFER!(aborting)" );
// std::terminate( );
// }
//if ( fmt_res == STRSAFE_E_INVALID_PARAMETER ) {
// displayWindowsMsgBoxWithMessage( L"Error in COwnerDrawnListCtrl::SortItems - StringCchPrintfW failed, STRSAFE_E_INVALID_PARAMETER!(aborting)" );
// std::terminate( );
// }
//else {
// displayWindowsMsgBoxWithMessage( L"Error in COwnerDrawnListCtrl::SortItems - StringCchPrintfW failed, unknown error!!(aborting)" );
// std::terminate( );
// }

}
Expand Down
10 changes: 10 additions & 0 deletions WinDirStat/windirstat/stdafx.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ static_assert( ITEM_ROW_HEIGHT > -1, "Rows need to be a positive size!" );
#endif


#ifndef WDS_ASSERT_NEVER_REACHED

//this line of code should NEVER be reached. ASSERT( false ) on reaching in Debug build.
#define WDS_ASSERT_NEVER_REACHED( ) ASSERT( false );
#else
#error already defined??!?
#endif



#ifndef DEBUG
//#pragma warning(3:4710) //The given function was selected for inline expansion, but the compiler did not perform the inlining.
#endif
Expand Down

0 comments on commit 113b38b

Please sign in to comment.