Skip to content

Commit

Permalink
add several safety/consistency and UI improvements
Browse files Browse the repository at this point in the history
HashCheck will never save a partially complete checksum file:
 * canceling an in-progress save causes the checksum file to be deleted
 * the Save button on file properties is unavailable if Stop is pressed

File read errors are now more visible:
 * in a checksum file, they are included as all 0's so that verify fails
 * on file properties they are included as all X's to stand out

When selected hashes in Options change, the tab is updated immediately:
 * any hashes which have already been calculated are not recalculated

Saving checksum files with an unselected hash from file properties works
  • Loading branch information
gurnec committed Aug 14, 2016
1 parent 6e8307d commit c8c09a8
Show file tree
Hide file tree
Showing 15 changed files with 424 additions and 185 deletions.
89 changes: 69 additions & 20 deletions HashCalc.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ __forceinline VOID WINAPI HashCalcSetSavePrefix( PHASHCALCCONTEXT phcctx, PTSTR
Path processing
\*============================================================================*/

VOID WINAPI HashCalcPrepare( PHASHCALCCONTEXT phcctx )
BOOL WINAPI HashCalcPrepare( PHASHCALCCONTEXT phcctx )
{
PTSTR pszPrev = NULL;
PTSTR pszCurrent, pszCurrentEnd;
Expand Down Expand Up @@ -128,7 +128,7 @@ VOID WINAPI HashCalcPrepare( PHASHCALCCONTEXT phcctx )

if (pItem)
{
pItem->bValid = FALSE;
pItem->results.dwFlags = 0;
pItem->cchPath = cchCurrent;
memcpy(pItem->szPath, pszCurrent, cbCurrent);

Expand All @@ -143,10 +143,11 @@ VOID WINAPI HashCalcPrepare( PHASHCALCCONTEXT phcctx )
if (phcctx->status == PAUSED)
WaitForSingleObject(phcctx->hUnpauseEvent, INFINITE);
if (phcctx->status == CANCEL_REQUESTED)
return;
return(FALSE);

pszPrev = pszCurrent;
}
return(TRUE);
}

VOID WINAPI HashCalcWalkDirectory( PHASHCALCCONTEXT phcctx, PTSTR pszPath, UINT cchPath )
Expand Down Expand Up @@ -192,7 +193,7 @@ VOID WINAPI HashCalcWalkDirectory( PHASHCALCCONTEXT phcctx, PTSTR pszPath, UINT

if (pItem)
{
pItem->bValid = FALSE;
pItem->results.dwFlags = 0;
pItem->cchPath = cchNew;
memcpy(pItem->szPath, pszPath, cbPathBuffer);

Expand Down Expand Up @@ -300,8 +301,6 @@ VOID WINAPI HashCalcInitSave( PHASHCALCCONTEXT phcctx )
phcctx->ofn.nFilterIndex &&
phcctx->ofn.nFilterIndex <= NUM_HASHES)
{
BOOL bSuccess = FALSE;

// Save the filter in the user's preferences
if (phcctx->opt.dwFilterIndex != phcctx->ofn.nFilterIndex)
{
Expand Down Expand Up @@ -330,7 +329,7 @@ VOID WINAPI HashCalcInitSave( PHASHCALCCONTEXT phcctx )
// Open the file for output
phcctx->hFileOut = CreateFile(
pszFile,
FILE_APPEND_DATA,
FILE_APPEND_DATA | DELETE,
FILE_SHARE_READ,
NULL,
CREATE_ALWAYS,
Expand Down Expand Up @@ -385,19 +384,32 @@ VOID WINAPI HashCalcSetSaveFormat( PHASHCALCCONTEXT phcctx )

BOOL WINAPI HashCalcWriteResult( PHASHCALCCONTEXT phcctx, PHASHCALCITEM pItem )
{
PCTSTR pszHash;
WCHAR szWbuffer[MAX_PATH_BUFFER];
CHAR szAbuffer[MAX_PATH_BUFFER];
PCTSTR pszHash; // will be pointed to the hash name
WCHAR szWbuffer[MAX_PATH_BUFFER]; // wide-char buffer
CHAR szAbuffer[MAX_PATH_BUFFER]; // narrow-char buffer
#ifdef UNICODE
# define szTbuffer szWbuffer
#else
# define szTbuffer szAbuffer
#endif
PVOID pvLine; // Will be pointed to the buffer to write out
size_t cchLine, cbLine; // Length of line, in TCHARs or BYTEs, EXCLUDING the terminator

if (!pItem->bValid)
return(FALSE);
PTSTR szTbufferAppend = szTbuffer; // current end of the buffer used to build output
size_t cchLine = MAX_PATH_BUFFER; // starts off as count of remaining TCHARS in the buffer
PVOID pvLine; // will be pointed to the buffer to write out
size_t cbLine; // will be line length in bytes, EXCLUDING nul terminator
BOOL bRetval = TRUE;

// If the checksum to save isn't present in the results
if (! ((1 << (phcctx->ofn.nFilterIndex - 1)) & pItem->results.dwFlags))
{
// Start with a commented-out error message - "; UNREADABLE:"
WCHAR szUnreadable[MAX_STRINGRES];
LoadString(g_hModThisDll, IDS_HV_STATUS_UNREADABLE, szUnreadable, MAX_STRINGRES);
StringCchPrintfEx(szTbufferAppend, cchLine, &szTbufferAppend, &cchLine, 0, TEXT("; %s:\r\n"), szUnreadable);

// We'll still output a hash, but it will be all 0's, that way Verify will indicate an mismatch
HashCalcClearInvalid(&pItem->results, TEXT('0'));
bRetval = FALSE;
}

// Translate the filter index to a hash
switch (phcctx->ofn.nFilterIndex)
Expand All @@ -409,19 +421,18 @@ BOOL WINAPI HashCalcWriteResult( PHASHCALCCONTEXT phcctx, PHASHCALCITEM pItem )
}

// Format the line
#define HashCalcFormat(a, b) StringCchPrintfEx(szTbuffer, MAX_PATH_BUFFER, NULL, &cchLine, 0, phcctx->szFormat, a, b)
#define HashCalcFormat(a, b) StringCchPrintfEx(szTbufferAppend, cchLine, &szTbufferAppend, &cchLine, 0, phcctx->szFormat, a, b)
(phcctx->ofn.nFilterIndex == 1) ?
HashCalcFormat(pItem->szPath + phcctx->cchAdjusted, pszHash) : // SFV
HashCalcFormat(pszHash, pItem->szPath + phcctx->cchAdjusted); // everything else
#undef HashCalcFormat
// cchLine is temporarily the count of characters left in the buffer instead of the line length

#ifdef _TIMED
StringCchPrintfEx(szTbuffer + (MAX_PATH_BUFFER-cchLine), cchLine, NULL, &cchLine, 0,
StringCchPrintfEx(szTbufferAppend, cchLine, NULL, &cchLine, 0,
_T("; Elapsed: %d ms\r\n"), pItem->dwElapsed);
#endif

cchLine = MAX_PATH_BUFFER - cchLine; // now it's back to being the line length
cchLine = MAX_PATH_BUFFER - cchLine; // from now on cchLine is the line length in bytes, EXCLUDING nul terminator
if (cchLine > 0)
{
// Convert to the correct encoding
Expand Down Expand Up @@ -479,7 +490,45 @@ BOOL WINAPI HashCalcWriteResult( PHASHCALCCONTEXT phcctx, PHASHCALCITEM pItem )
}
else return(FALSE);

return(TRUE);
return(bRetval);
}

VOID WINAPI HashCalcClearInvalid( PWHRESULTEX pwhres, WCHAR cInvalid )
{
#ifdef UNICODE
# define _tmemset wmemset
#else
# define _tmemset memset
#endif

#define HASH_CLEAR_INVALID_op(alg) \
if (! (pwhres->dwFlags & WHEX_CHECK##alg)) \
{ \
_tmemset(pwhres->szHex##alg, cInvalid, countof(pwhres->szHex##alg) - 1); \
pwhres->szHex##alg[countof(pwhres->szHex##alg) - 1] = L'\0'; \
}
FOR_EACH_HASH(HASH_CLEAR_INVALID_op)
}

// This can only succeed on Windows Vista and later;
// returns FALSE on failure
BOOL WINAPI HashCalcDeleteFileByHandle(HANDLE hFile)
{
if (hFile == INVALID_HANDLE_VALUE)
return(FALSE);

HMODULE hKernel32 = GetModuleHandle(TEXT("kernel32.dll"));
if (hKernel32 == NULL)
return(FALSE);

typedef BOOL(WINAPI* PFN_SFIBH)(_In_ HANDLE, _In_ FILE_INFO_BY_HANDLE_CLASS, _In_ LPVOID, _In_ DWORD);
PFN_SFIBH pfnSetFileInformationByHandle = (PFN_SFIBH)GetProcAddress(hKernel32, "SetFileInformationByHandle");
if (pfnSetFileInformationByHandle == NULL)
return(FALSE);

FILE_DISPOSITION_INFO fdi;
fdi.DeleteFile = TRUE;
return(pfnSetFileInformationByHandle(hFile, FileDispositionInfo, &fdi, sizeof(fdi)));
}

VOID WINAPI HashCalcSetSavePrefix( PHASHCALCCONTEXT phcctx, PTSTR pszSave )
Expand Down
5 changes: 3 additions & 2 deletions HashCalc.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ typedef struct {

// Per-file data
typedef struct {
BOOL bValid; // FALSE if the file could not be opened
UINT cchPath; // length of path in characters, not including NULL
WHRESULTEX results; // hash results
#ifdef _TIMED
Expand All @@ -101,10 +100,12 @@ typedef struct {
} HASHCALCITEM, *PHASHCALCITEM;

// Public functions
VOID WINAPI HashCalcPrepare( PHASHCALCCONTEXT phcctx );
BOOL WINAPI HashCalcPrepare( PHASHCALCCONTEXT phcctx );
VOID WINAPI HashCalcInitSave( PHASHCALCCONTEXT phcctx );
VOID WINAPI HashCalcSetSaveFormat( PHASHCALCCONTEXT phcctx );
BOOL WINAPI HashCalcWriteResult( PHASHCALCCONTEXT phcctx, PHASHCALCITEM pItem );
VOID WINAPI HashCalcClearInvalid( PWHRESULTEX pwhres, WCHAR cInvalid );
BOOL WINAPI HashCalcDeleteFileByHandle( HANDLE hFile );
VOID WINAPI HashCalcTogglePrep( PHASHCALCCONTEXT phcctx, BOOL bState );

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion HashCheck.rc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ IDD_OPTIONS DIALOGEX 10, 10, 200, 264
AUTORADIOBUTTON "", IDC_OPT_ENCODING_UTF16, 13, 99, 174, 10
AUTORADIOBUTTON "", IDC_OPT_ENCODING_ANSI, 13, 113, 174, 10
GROUPBOX "", IDC_OPT_CHK, 7, 137, 186, 58, WS_GROUP
AUTOCHECKBOX "CRC-32",IDC_OPT_CHK_CRC32, 13, 150, 54, 10, WS_TABSTOP
AUTOCHECKBOX "C&RC-32",IDC_OPT_CHK_CRC32, 13, 150, 54, 10, WS_TABSTOP
AUTOCHECKBOX "MD5",IDC_OPT_CHK_MD5, 13, 164, 54, 10, WS_TABSTOP
AUTOCHECKBOX "SHA-1",IDC_OPT_CHK_SHA1, 13, 178, 54, 10, WS_TABSTOP
AUTOCHECKBOX "SHA-256",IDC_OPT_CHK_SHA256, 100, 150, 54, 10, WS_TABSTOP
Expand Down
48 changes: 34 additions & 14 deletions HashCheckCommon.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ HANDLE __fastcall CreateThreadCRT( PVOID pThreadProc, PVOID pvParam )
pcmnctx->cHandledMsgs = 0;
pcmnctx->hWndPBTotal = GetDlgItem(pcmnctx->hWnd, IDC_PROG_TOTAL);
pcmnctx->hWndPBFile = GetDlgItem(pcmnctx->hWnd, IDC_PROG_FILE);
if (pcmnctx->hUnpauseEvent == NULL)
pcmnctx->hUnpauseEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
SendMessage(pcmnctx->hWndPBFile, PBM_SETRANGE, 0, MAKELPARAM(0, PROGRESS_BAR_STEPS));

pThreadProc = WorkerThreadStartup;
Expand Down Expand Up @@ -138,7 +140,8 @@ VOID WINAPI SetProgressBarPause( PCOMMONCONTEXT pcmnctx, WPARAM iState )
// Vista's progress bar is buggy--if you pause it while it is animating,
// the color will not change (but it will stop animating), so it may
// be necessary to send another PBM_SETSTATE to get it right
SetTimer(pcmnctx->hWnd, TIMER_ID_PAUSE, 750, NULL);
if (iState == PBST_PAUSED)
SetTimer(pcmnctx->hWnd, TIMER_ID_PAUSE, 750, NULL);
}
}

Expand Down Expand Up @@ -188,7 +191,7 @@ VOID WINAPI WorkerThreadStop( PCOMMONCONTEXT pcmnctx )
}

// Disable the control buttons
if (!(pcmnctx->dwFlags & HCF_EXIT_PENDING))
if (! (pcmnctx->dwFlags & (HCF_EXIT_PENDING | HCF_RESTARTING)))
{
EnableWindow(GetDlgItem(pcmnctx->hWnd, IDC_PAUSE), FALSE);
EnableWindow(GetDlgItem(pcmnctx->hWnd, IDC_STOP), FALSE);
Expand All @@ -203,11 +206,11 @@ VOID WINAPI WorkerThreadCleanup( PCOMMONCONTEXT pcmnctx )
// There are only two times this function gets called:
// Case 1: The worker thread has exited on its own, and this function
// was invoked in response to the thread's exit notification.
// Case 2: A forced abort was requested (app exit, system shutdown, etc.),
// Case 2: A forced abort was requested (app exit, settings change, etc.),
// where this is called right after calling WorkerThreadStop to signal the
// thread to exit.

if (pcmnctx->hThread)
if (pcmnctx->hThread != NULL)
{
if (pcmnctx->status != INACTIVE)
{
Expand All @@ -220,12 +223,19 @@ VOID WINAPI WorkerThreadCleanup( PCOMMONCONTEXT pcmnctx )
}

CloseHandle(pcmnctx->hThread);
CloseHandle(pcmnctx->hUnpauseEvent);
pcmnctx->hThread = NULL;
}

// If we're done with the Unpause event and it's open, close it
if (!(pcmnctx->dwFlags & HCF_RESTARTING) && pcmnctx->hUnpauseEvent != NULL)
{
CloseHandle(pcmnctx->hUnpauseEvent);
pcmnctx->hUnpauseEvent = NULL;
}

pcmnctx->status = CLEANUP_COMPLETED;

if (!(pcmnctx->dwFlags & HCF_EXIT_PENDING))
if (! (pcmnctx->dwFlags & (HCF_EXIT_PENDING | HCF_RESTARTING)))
{
static const UINT16 arCtrls[] =
{
Expand All @@ -244,13 +254,11 @@ VOID WINAPI WorkerThreadCleanup( PCOMMONCONTEXT pcmnctx )

DWORD WINAPI WorkerThreadStartup( PCOMMONCONTEXT pcmnctx )
{
pcmnctx->hUnpauseEvent = CreateEvent(NULL, TRUE, TRUE, NULL);

pcmnctx->pfnWorkerMain(pcmnctx);

pcmnctx->status = INACTIVE;

if (!(pcmnctx->dwFlags & HCF_EXIT_PENDING))
if (! (pcmnctx->dwFlags & (HCF_EXIT_PENDING | HCF_RESTARTING)))
PostMessage(pcmnctx->hWnd, HM_WORKERTHREAD_DONE, (WPARAM)pcmnctx, 0);

return(0);
Expand Down Expand Up @@ -323,7 +331,7 @@ __inline VOID UpdateProgressBar( HWND hWndPBFile, PCRITICAL_SECTION pCritSec,
}
}

VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath, PBOOL pbSuccess,
VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath,
PWHCTXEX pwhctx, PWHRESULTEX pwhres, PBYTE pbuffer,
PFILESIZE pFileSize, LPARAM lParam,
PCRITICAL_SECTION pUpdateCritSec, volatile ULONGLONG* pcbCurrentMaxSize
Expand All @@ -334,8 +342,6 @@ VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath, PBOOL
{
HANDLE hFile;

*pbSuccess = FALSE;

// If the worker thread is working so fast that the UI cannot catch up,
// pause for a bit to let things settle down
while (pcmnctx->cSentMsgs > pcmnctx->cHandledMsgs + 50)
Expand All @@ -347,6 +353,17 @@ VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath, PBOOL
return;
}

// This can happen if a user changes the hash selection in HashProp (if no
// new hashes were selected; we still want to run the throttling code above)
if (pwhctx->dwFlags == 0)
{
#ifdef _TIMED
if (pdwElapsed)
*pdwElapsed = 0;
#endif
return;
}

// Indicate that we want lower-case results (TODO: make this an option)
pwhctx->uCaseMode = WHFMT_LOWERCASE;

Expand Down Expand Up @@ -413,8 +430,11 @@ VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath, PBOOL
if (pdwElapsed)
*pdwElapsed = GetTickCount() - dwStarted;
#endif
if (cbFileRead == cbFileSize)
*pbSuccess = TRUE;
// If we encountered a file read error
if (cbFileRead != cbFileSize)
// Clear the valid-results bits for the hashes we just calculated
// (they are set by WHFinishEx, but they're apparently *not* valid)
pwhres->dwFlags &= ~pwhctx->dwFlags;

if (bUpdateProgress)
UpdateProgressBar(pcmnctx->hWndPBFile, pUpdateCritSec, &bCurrentlyUpdating,
Expand Down
17 changes: 10 additions & 7 deletions HashCheckCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@ extern "C" {
#define THREAD_SUSPEND_ERROR ((DWORD)-1)
#define TIMER_ID_PAUSE 1

// Flags
#define HCF_EXIT_PENDING 0x0001
#define HCF_MARQUEE 0x0002
#define HVF_HAS_SET_TYPE 0x0004
#define HVF_ITEM_HILITE 0x0008
#define HPF_HAS_RESIZED 0x0004
// Flags of DWORD width (which is an unsigned long)
#define HCF_EXIT_PENDING 0x0001UL
#define HCF_MARQUEE 0x0002UL
#define HCF_RESTARTING 0x0004UL
#define HVF_HAS_SET_TYPE 0x0008UL
#define HVF_ITEM_HILITE 0x0010UL
#define HPF_HAS_RESIZED 0x0008UL
#define HPF_HLIST_PREPPED 0x0010UL
#define HPF_INTERRUPTED 0x0020UL

// Messages
#define HM_WORKERTHREAD_DONE (WM_APP + 0) // wParam = ctx, lParam = 0
Expand Down Expand Up @@ -100,7 +103,7 @@ VOID WINAPI WorkerThreadCleanup( PCOMMONCONTEXT pcmnctx );

// Worker thread functions
DWORD WINAPI WorkerThreadStartup( PCOMMONCONTEXT pcmnctx );
VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath, PBOOL pbSuccess,
VOID WINAPI WorkerThreadHashFile( PCOMMONCONTEXT pcmnctx, PCTSTR pszPath,
PWHCTXEX pwhctx, PWHRESULTEX pwhres, PBYTE pbuffer,
PFILESIZE pFileSize, LPARAM lParam,
PCRITICAL_SECTION pUpdateCritSec, volatile ULONGLONG* pcbCurrentMaxSize
Expand Down
2 changes: 1 addition & 1 deletion HashCheckTranslations.rc
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ STRINGTABLE LANGUAGE LANG_CZECH, SUBLANG_DEFAULT
IDS_OPT_TITLE "HashCheck nastavení"
IDS_OPT_CM "Integrace"
IDS_OPT_CM_ALWAYS "&Zobraz v kontextovém menu"
IDS_OPT_CM_EXTENDED "Zobraz v &rozšířeném kontextovém menu"
IDS_OPT_CM_EXTENDED "Zobraz v rozšířeném &kontextovém menu"
IDS_OPT_CM_NEVER "&Nezobrazuj v kontextovém menu"
IDS_OPT_ENCODING "Znaková sada souboru kontrolního součtu"
IDS_OPT_ENCODING_UTF8 "Ulož v U&TF-8"
Expand Down
Loading

0 comments on commit c8c09a8

Please sign in to comment.