From 8a33307b6c4aeac4172d88fbd25409e62474e13e Mon Sep 17 00:00:00 2001 From: Andrey Prygunkov Date: Tue, 25 Dec 2007 14:20:27 +0000 Subject: [PATCH] improved error-reporting (_brokenlog.txt) on crc-errors --- ArticleDownloader.cpp | 90 ++++++++++++++++++++----------------------- ArticleDownloader.h | 3 +- Decoder.cpp | 55 ++------------------------ Decoder.h | 43 ++++++++++----------- QueueCoordinator.cpp | 1 + 5 files changed, 67 insertions(+), 125 deletions(-) diff --git a/ArticleDownloader.cpp b/ArticleDownloader.cpp index 6c1f8493..cb748066 100644 --- a/ArticleDownloader.cpp +++ b/ArticleDownloader.cpp @@ -65,7 +65,6 @@ ArticleDownloader::ArticleDownloader() m_szArticleFilename = NULL; m_szInfoName = NULL; m_pConnection = NULL; - m_pDecoder = NULL; m_eStatus = adUndefined; m_iBytes = 0; memset(&m_tStartTime, 0, sizeof(m_tStartTime)); @@ -88,10 +87,6 @@ ArticleDownloader::~ArticleDownloader() { free(m_szInfoName); } - if (m_pDecoder) - { - delete m_pDecoder; - } } void ArticleDownloader::SetTempFilename(const char* v) @@ -191,7 +186,8 @@ void ArticleDownloader::Run() FreeConnection(); } - if ((Status == adFailed) && ((retry > 1) || !connected) && !IsStopped()) + if ((Status == adFailed || (Status == adCrcError && g_pOptions->GetRetryOnCrcError())) && + ((retry > 1) || !connected) && !IsStopped()) { info("Waiting %i sec to retry", g_pOptions->GetRetryInterval()); int msec = 0; @@ -207,8 +203,9 @@ void ArticleDownloader::Run() Status = adFailed; break; } - - if ((Status == adFinished) || (Status == adFatalError)) + + if ((Status == adFinished) || (Status == adFatalError) || + (Status == adCrcError && !g_pOptions->GetRetryOnCrcError())) { break; } @@ -228,7 +225,7 @@ void ArticleDownloader::Run() { if (iMaxLevel > 0) { - warn("Aticle %s @ all servers failed: Article not found", m_szInfoName); + warn("Article %s @ all servers failed: Article not found", m_szInfoName); } break; } @@ -268,12 +265,12 @@ void ArticleDownloader::Run() SetStatus(Status); - debug("Existing ArticleDownloader-loop"); + debug("Exiting ArticleDownloader-loop"); } ArticleDownloader::EStatus ArticleDownloader::Download() { - // at first, change group! dryan's level wants it this way... ;-) + // at first, change group bool grpchanged = false; for (FileInfo::Groups::iterator it = m_pFileInfo->GetGroups()->begin(); it != m_pFileInfo->GetGroups()->end(); it++) { @@ -404,49 +401,42 @@ ArticleDownloader::EStatus ArticleDownloader::Download() FreeConnection(); if ((g_pOptions->GetDecoder() == Options::dcUulib) || - (g_pOptions->GetDecoder() == Options::dcYenc)) + (g_pOptions->GetDecoder() == Options::dcYenc)) { - // Give time to other threads. Help to avoid hangs on Asus WL500g router. + // Give time to other threads usleep(10 * 1000); SetStatus(adDecoding); struct _timeval StartTime, EndTime; gettimeofday(&StartTime, 0); - bool OK = false; + + Decoder decoder; if (g_pOptions->GetDecoder() == Options::dcUulib) { - m_pDecoder = new Decoder(); - m_pDecoder->SetKind(Decoder::dcUulib); + decoder.SetKind(Decoder::dcUulib); } else if (g_pOptions->GetDecoder() == Options::dcYenc) { - m_pDecoder = new Decoder(); - m_pDecoder->SetKind(Decoder::dcYenc); + decoder.SetKind(Decoder::dcYenc); } - if (m_pDecoder) - { - m_pDecoder->SetSrcFilename(dnfilename); + decoder.SetSrcFilename(dnfilename); + char tmpdestfile[1024]; + snprintf(tmpdestfile, 1024, "%s.dec", m_szResultFilename); + tmpdestfile[1024-1] = '\0'; + decoder.SetDestFilename(tmpdestfile); - char tmpdestfile[1024]; - snprintf(tmpdestfile, 1024, "%s.dec", m_szResultFilename); - tmpdestfile[1024-1] = '\0'; - - m_pDecoder->SetDestFilename(tmpdestfile); - OK = m_pDecoder->Execute(); - if (OK) - { - rename(tmpdestfile, m_szResultFilename); - } - else - { - remove(tmpdestfile); - } - if (m_pDecoder->GetArticleFilename()) - { - m_szArticleFilename = strdup(m_pDecoder->GetArticleFilename()); - } - delete m_pDecoder; - m_pDecoder = NULL; + bool bOK = decoder.Execute(); + if (bOK) + { + rename(tmpdestfile, m_szResultFilename); + } + else + { + remove(tmpdestfile); + } + if (decoder.GetArticleFilename()) + { + m_szArticleFilename = strdup(decoder.GetArticleFilename()); } gettimeofday(&EndTime, 0); @@ -456,7 +446,7 @@ ArticleDownloader::EStatus ArticleDownloader::Download() #else float fDeltaTime = ((EndTime.tv_sec - StartTime.tv_sec) * 1000000 + (EndTime.tv_usec - StartTime.tv_usec)) / 1000.0; #endif - if (OK) + if (bOK) { info("Successfully downloaded %s", m_szInfoName); debug("Decode time %.1f ms", fDeltaTime); @@ -464,9 +454,17 @@ ArticleDownloader::EStatus ArticleDownloader::Download() } else { - warn("Decoding %s failed", m_szInfoName); remove(m_szResultFilename); - return adFailed; + if (decoder.GetCrcError()) + { + warn("Decoding %s failed: CRC-Error", m_szInfoName); + return adCrcError; + } + else + { + warn("Decoding %s failed", m_szInfoName); + return adFailed; + } } } else if (g_pOptions->GetDecoder() == Options::dcNone) @@ -494,10 +492,6 @@ void ArticleDownloader::LogDebugInfo() #endif debug(" Download: status=%s, LastUpdateTime=%s, filename=%s", GetStatusText(), szTime, BaseFileName(GetTempFilename())); - if (m_pDecoder) - { - m_pDecoder->LogDebugInfo(); - } } void ArticleDownloader::Stop() diff --git a/ArticleDownloader.h b/ArticleDownloader.h index 398430aa..2bbf5f18 100644 --- a/ArticleDownloader.h +++ b/ArticleDownloader.h @@ -36,7 +36,6 @@ #include "DownloadInfo.h" #include "Thread.h" #include "NNTPConnection.h" -#include "Decoder.h" class ArticleDownloader : public Thread, public Subject { @@ -47,6 +46,7 @@ public: adRunning, adFinished, adFailed, + adCrcError, adDecoding, adJoining, adNotFound, @@ -64,7 +64,6 @@ private: char* m_szArticleFilename; char* m_szInfoName; time_t m_tLastUpdateTime; - Decoder* m_pDecoder; Semaphore m_semInitialized; Semaphore m_semWaited; static const char* m_szJobStatus[]; diff --git a/Decoder.cpp b/Decoder.cpp index d23003cf..2d247307 100644 --- a/Decoder.cpp +++ b/Decoder.cpp @@ -48,20 +48,10 @@ #include #endif -//#define USEEXTERNALDECODER // not working -//#define DEBUGDECODER - #include "Decoder.h" #include "Log.h" -#include "Options.h" #include "Util.h" -extern Options* g_pOptions; - -#ifdef DEBUGDECODER -int g_iDecoderID = 0; -#endif - Mutex Decoder::m_mutexDecoder; unsigned int Decoder::crc_tab[256]; @@ -84,8 +74,7 @@ Decoder::Decoder() m_szDestFilename = NULL; m_szArticleFilename = NULL; m_eKind = dcYenc; - m_iDebugStatus = 0; - m_iDebugLines = 0; + m_bCrcError = false; } Decoder::~ Decoder() @@ -120,11 +109,6 @@ bool Decoder::DecodeUulib() m_mutexDecoder.Lock(); -#ifdef DEBUGDECODER - debug("Decoding ID %i (%s)", g_iDecoderID, szSrcFilename); -#endif - -#ifndef USEEXTERNALDECODER UUInitialize(); UUSetOption(UUOPT_DESPERATE, 1, NULL); @@ -186,13 +170,6 @@ bool Decoder::DecodeUulib() } UUCleanUp(); -#else - execl("/usr/local/bin", "uudeview", szSrcFilename, szDestFilename); -#endif - -#ifdef DEBUGDECODER - debug("Finished decoding ID %i (%s)", g_iDecoderID++, szDestFilename); -#endif m_mutexDecoder.Unlock(); @@ -227,10 +204,7 @@ bool Decoder::DecodeYenc() bool end = false; unsigned long expectedCRC = 0; unsigned long calculatedCRC = 0xFFFFFFFF; - m_iDebugStatus = 1; bool eof = !fgets(buffer, sizeof(buffer), infile); - m_iDebugLines++; - m_iDebugStatus = 2; while (!eof) { if (body) @@ -238,7 +212,6 @@ bool Decoder::DecodeYenc() if (strstr(buffer, "=yend size=")) { end = true; - m_iDebugStatus = 3; char* pc = strstr(buffer, "pcrc32="); if (pc) { @@ -247,7 +220,6 @@ bool Decoder::DecodeYenc() } break; } - m_iDebugStatus = 4; char* iptr = buffer; char* optr = buffer; while (*iptr) @@ -269,57 +241,42 @@ bool Decoder::DecodeYenc() } iptr++; } - m_iDebugStatus = 5; calculatedCRC = crc32m(calculatedCRC, (unsigned char *)buffer, optr - buffer); fwrite(buffer, 1, optr - buffer, outfile); - m_iDebugStatus = 6; } else { if (strstr(buffer, "=ypart begin=")) { - m_iDebugStatus = 7; body = true; } else if (strstr(buffer, "=ybegin part=")) { - m_iDebugStatus = 8; char* pb = strstr(buffer, "name="); if (pb) { - m_iDebugStatus = 9; pb += 5; //=strlen("name=") char* pe; for (pe = pb; *pe != '\0' && *pe != '\n' && *pe != '\r'; pe++) ; m_szArticleFilename = (char*)malloc(pe - pb + 1); strncpy(m_szArticleFilename, pb, pe - pb); m_szArticleFilename[pe - pb] = '\0'; - m_iDebugStatus = 10; } - m_iDebugStatus = 11; } } - m_iDebugStatus = 12; eof = !fgets(buffer, sizeof(buffer), infile); - m_iDebugStatus = 13; - m_iDebugLines++; } - m_iDebugStatus = 14; calculatedCRC ^= 0xFFFFFFFF; debug("Expected pcrc32=%x", expectedCRC); debug("Calculated pcrc32=%x", calculatedCRC); - bool CrcOK = expectedCRC == calculatedCRC; - if (!CrcOK) - { - warn("CRC-Error for \"%s\"", m_szDestFilename); - } + m_bCrcError = expectedCRC != calculatedCRC; fclose(infile); fclose(outfile); - return body && end && (CrcOK || !g_pOptions->GetRetryOnCrcError()); + return body && end && !m_bCrcError; } /* from crc32.c (http://www.koders.com/c/fid699AFE0A656F0022C9D6B9D1743E697B69CE5815.aspx) @@ -378,9 +335,3 @@ unsigned long Decoder::crc32m(unsigned long startCrc, unsigned char *block, unsi } return crc; } - -void Decoder::LogDebugInfo() -{ - debug(" Decoder: status=%i, lines=%i, filename=%s, ArticleFileName=%s", - m_iDebugStatus, m_iDebugLines, BaseFileName(m_szSrcFilename), m_szArticleFilename); -} diff --git a/Decoder.h b/Decoder.h index 6e51213d..7d79ad3e 100644 --- a/Decoder.h +++ b/Decoder.h @@ -29,8 +29,6 @@ #include "Thread.h" -//#define DECODER_INTERNAL_FGETS - class Decoder { public: @@ -41,32 +39,31 @@ public: }; private: - static Mutex m_mutexDecoder; + static Mutex m_mutexDecoder; static unsigned int crc_tab[256]; - EKind m_eKind; - const char* m_szSrcFilename; - const char* m_szDestFilename; - char* m_szArticleFilename; - int m_iDebugStatus; - int m_iDebugLines; + EKind m_eKind; + const char* m_szSrcFilename; + const char* m_szDestFilename; + char* m_szArticleFilename; + bool m_bCrcError; - bool DecodeUulib(); - bool DecodeYenc(); - static void crc32gentab(); - unsigned long crc32m(unsigned long startCrc, unsigned char *block, unsigned int length); + bool DecodeUulib(); + bool DecodeYenc(); + static void crc32gentab(); + unsigned long crc32m(unsigned long startCrc, unsigned char *block, unsigned int length); public: - Decoder(); - ~Decoder(); - bool Execute(); - void SetKind(EKind eKind) { m_eKind = eKind; } - void SetSrcFilename(const char* szSrcFilename) { m_szSrcFilename = szSrcFilename; } - void SetDestFilename(const char* szDestFilename) { m_szDestFilename = szDestFilename; } - const char* GetArticleFilename() { return m_szArticleFilename; } - void LogDebugInfo(); + Decoder(); + ~Decoder(); + bool Execute(); + void SetKind(EKind eKind) { m_eKind = eKind; } + void SetSrcFilename(const char* szSrcFilename) { m_szSrcFilename = szSrcFilename; } + void SetDestFilename(const char* szDestFilename) { m_szDestFilename = szDestFilename; } + const char* GetArticleFilename() { return m_szArticleFilename; } + bool GetCrcError() { return m_bCrcError; } - static void Init(); - static void Final(); + static void Init(); + static void Final(); }; #endif diff --git a/QueueCoordinator.cpp b/QueueCoordinator.cpp index 30a70b24..3f643597 100644 --- a/QueueCoordinator.cpp +++ b/QueueCoordinator.cpp @@ -46,6 +46,7 @@ #include "ArticleDownloader.h" #include "Log.h" #include "Util.h" +#include "Decoder.h" extern Options* g_pOptions; extern ServerPool* g_pServerPool;