From f37b387cc602acf144d4c901fd639e0a92d03560 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Sat, 26 Dec 2015 11:44:47 -0800 Subject: [PATCH] Fix handling of entries with missing threads When GSHK adds files to an archive, it doesn't create threads for zero-length data and resource forks. NufxLib had a workaround for this, but it wasn't handling all possible cases. We now fully handle "Miranda threads" (if you cannot afford a thread, one will be provided for you). This broke test-basic, because a callback gets called one extra time now due to the additional thread. It also broke test-twirl, which uses "mask dataless" and is sensitive to the order in which threads appear. (test-twirl actually works just fine, but the CRC check is too simple-minded, and is arguably incorrect.) Since this can apparently break things, I'm making this a minor version bump, to 3.1.0-a1. I also tweaked the NuLib2 file listing to test for the extended file storage type, rather than simply scanning for data threads. Forked files are now listed as such, even when they're missing the actual resource fork data thread. --- nufxlib/ChangeLog.txt | 5 ++- nufxlib/FileIO.c | 7 +++ nufxlib/NufxLib.h | 11 +++-- nufxlib/Record.c | 87 ++++++++++++++++++++++--------------- nufxlib/Thread.c | 54 +++++++++++++---------- nufxlib/samples/TestBasic.c | 10 +++-- nulib2/ChangeLog.txt | 5 ++- nulib2/List.c | 22 ++++++++-- nulib2/Main.c | 2 +- nulib2/State.c | 2 +- 10 files changed, 133 insertions(+), 72 deletions(-) diff --git a/nufxlib/ChangeLog.txt b/nufxlib/ChangeLog.txt index 6ec7872..3c33730 100644 --- a/nufxlib/ChangeLog.txt +++ b/nufxlib/ChangeLog.txt @@ -1,4 +1,7 @@ -2005/01/09 ***** v3.0.0 shipped ***** +2015/12/26 fadden + - Fix handling of entries with missing threads. + +2015/01/09 ***** v3.0.0 shipped ***** 2015/01/03 fadden - Mac OS X: replace Carbon FinderInfo calls with BSD xattr. diff --git a/nufxlib/FileIO.c b/nufxlib/FileIO.c index 9d20366..1e44984 100644 --- a/nufxlib/FileIO.c +++ b/nufxlib/FileIO.c @@ -228,6 +228,13 @@ typedef struct NuFileInfo { /* * Determine whether the record has both data and resource forks. + * + * TODO: if we're not using "mask dataless", scanning threads may not + * get the right answer, because GSHK omits theads for zero-length forks. + * We could check pRecord->recStorageType, though we have to be careful + * because that's overloaded for disk images. In any event, the result + * from this method isn't relevant unless we're trying to use forked + * files on the native filesystem. */ static Boolean Nu_IsForkedFile(NuArchive* pArchive, const NuRecord* pRecord) { diff --git a/nufxlib/NufxLib.h b/nufxlib/NufxLib.h index 1ec1ada..349917c 100644 --- a/nufxlib/NufxLib.h +++ b/nufxlib/NufxLib.h @@ -33,7 +33,7 @@ extern "C" { * fixes. Unless, of course, your code depends upon that fix. */ #define kNuVersionMajor 3 -#define kNuVersionMinor 0 +#define kNuVersionMinor 1 #define kNuVersionBug 0 @@ -201,13 +201,16 @@ typedef uint32_t NuThreadID; #define kNuThreadClassControl 0x0001 #define kNuThreadClassData 0x0002 #define kNuThreadClassFilename 0x0003 +#define kNuThreadKindDataFork 0x0000 /* when class=data */ +#define kNuThreadKindDiskImage 0x0001 /* when class=data */ +#define kNuThreadKindRsrcFork 0x0002 /* when class=data */ #define kNuThreadIDOldComment NuMakeThreadID(kNuThreadClassMessage, 0x0000) #define kNuThreadIDComment NuMakeThreadID(kNuThreadClassMessage, 0x0001) #define kNuThreadIDIcon NuMakeThreadID(kNuThreadClassMessage, 0x0002) #define kNuThreadIDMkdir NuMakeThreadID(kNuThreadClassControl, 0x0000) -#define kNuThreadIDDataFork NuMakeThreadID(kNuThreadClassData, 0x0000) -#define kNuThreadIDDiskImage NuMakeThreadID(kNuThreadClassData, 0x0001) -#define kNuThreadIDRsrcFork NuMakeThreadID(kNuThreadClassData, 0x0002) +#define kNuThreadIDDataFork NuMakeThreadID(kNuThreadClassData, kNuThreadKindDataFork) +#define kNuThreadIDDiskImage NuMakeThreadID(kNuThreadClassData, kNuThreadKindDiskImage) +#define kNuThreadIDRsrcFork NuMakeThreadID(kNuThreadClassData, kNuThreadKindRsrcFork) #define kNuThreadIDFilename NuMakeThreadID(kNuThreadClassFilename, 0x0000) #define kNuThreadIDWildcard NuMakeThreadID(0xffff, 0xffff) diff --git a/nufxlib/Record.c b/nufxlib/Record.c index d77899c..858cf4d 100644 --- a/nufxlib/Record.c +++ b/nufxlib/Record.c @@ -1524,7 +1524,7 @@ NuError Nu_StreamExtract(NuArchive* pArchive) { NuError err = kNuErrNone; NuRecord tmpRecord; - Boolean hasInterestingThread; + Boolean needFakeData, needFakeRsrc; uint32_t count; long idx; @@ -1573,7 +1573,8 @@ NuError Nu_StreamExtract(NuArchive* pArchive) /*Nu_DebugDumpRecord(&tmpRecord); printf("\n");*/ - hasInterestingThread = false; + needFakeData = true; + needFakeRsrc = (tmpRecord.recStorageType == kNuStorageExtended); /* extract all relevant (remaining) threads */ pArchive->lastFileCreatedUNI = NULL; @@ -1581,7 +1582,11 @@ NuError Nu_StreamExtract(NuArchive* pArchive) const NuThread* pThread = Nu_GetThread(&tmpRecord, idx); if (pThread->thThreadClass == kNuThreadClassData) { - hasInterestingThread = true; + if (pThread->thThreadKind == kNuThreadKindDataFork) { + needFakeData = false; + } else if (pThread->thThreadKind == kNuThreadKindRsrcFork) { + needFakeRsrc = false; + } err = Nu_ExtractThreadBulk(pArchive, &tmpRecord, pThread); if (err == kNuErrSkipped) { err = Nu_SkipThread(pArchive, &tmpRecord, pThread); @@ -1595,7 +1600,8 @@ NuError Nu_StreamExtract(NuArchive* pArchive) if (NuGetThreadID(pThread) != kNuThreadIDComment && NuGetThreadID(pThread) != kNuThreadIDFilename) { - hasInterestingThread = true; + /* unknown stuff in record, skip thread fakery */ + needFakeData = needFakeRsrc = false; } err = Nu_SkipThread(pArchive, &tmpRecord, pThread); BailError(err); @@ -1603,19 +1609,19 @@ NuError Nu_StreamExtract(NuArchive* pArchive) } /* - * If we're trying to be compatible with ShrinkIt, and the record - * had nothing in it but comments and filenames, then we need to - * create a zero-byte data file (and possibly a resource fork). - * - * See notes in previous instance, above. + * As in Nu_ExtractRecordByPtr, we need to synthesize empty forks for + * cases where GSHK omitted the data thread entirely. */ - if (/*pArchive->valMaskDataless &&*/ !hasInterestingThread) { - err = Nu_FakeZeroExtract(pArchive, &tmpRecord, 0x0000); + Assert(!pArchive->valMaskDataless || (!needFakeData && !needFakeRsrc)); + if (needFakeData) { + err = Nu_FakeZeroExtract(pArchive, &tmpRecord, + kNuThreadKindDataFork); + BailError(err); + } + if (needFakeRsrc) { + err = Nu_FakeZeroExtract(pArchive, &tmpRecord, + kNuThreadKindRsrcFork); BailError(err); - if (tmpRecord.recStorageType == kNuStorageExtended) { - err = Nu_FakeZeroExtract(pArchive, &tmpRecord, 0x0002); - BailError(err); - } } /* dispose of the entry */ @@ -1698,20 +1704,26 @@ bail: static NuError Nu_ExtractRecordByPtr(NuArchive* pArchive, NuRecord* pRecord) { NuError err = kNuErrNone; - Boolean hasInterestingThread; + Boolean needFakeData, needFakeRsrc; uint32_t idx; + needFakeData = true; + needFakeRsrc = (pRecord->recStorageType == kNuStorageExtended); + Assert(!Nu_IsStreaming(pArchive)); /* we don't skip things we don't read */ Assert(pRecord != NULL); /* extract all relevant threads */ - hasInterestingThread = false; pArchive->lastFileCreatedUNI = NULL; for (idx = 0; idx < pRecord->recTotalThreads; idx++) { const NuThread* pThread = Nu_GetThread(pRecord, idx); if (pThread->thThreadClass == kNuThreadClassData) { - hasInterestingThread = true; + if (pThread->thThreadKind == kNuThreadKindDataFork) { + needFakeData = false; + } else if (pThread->thThreadKind == kNuThreadKindRsrcFork) { + needFakeRsrc = false; + } err = Nu_ExtractThreadBulk(pArchive, pRecord, pThread); if (err == kNuErrSkipped) { err = Nu_SkipThread(pArchive, pRecord, pThread); @@ -1722,7 +1734,13 @@ static NuError Nu_ExtractRecordByPtr(NuArchive* pArchive, NuRecord* pRecord) if (NuGetThreadID(pThread) != kNuThreadIDComment && NuGetThreadID(pThread) != kNuThreadIDFilename) { - hasInterestingThread = true; + /* + * This record has a thread we don't recognize. Disable + * the thread fakery to avoid doing anything weird -- we + * should only need to create zero-length files for + * simple file records. + */ + needFakeData = needFakeRsrc = false; } DBUG(("IGNORING 0x%08lx from '%s'\n", NuMakeThreadID(pThread->thThreadClass, pThread->thThreadKind), @@ -1731,28 +1749,27 @@ static NuError Nu_ExtractRecordByPtr(NuArchive* pArchive, NuRecord* pRecord) } /* - * If we're trying to be compatible with ShrinkIt, and the record - * had nothing in it but comments and filenames, then we need to - * create a zero-byte file. + * GSHK creates empty threads for zero-length forks. It doesn't always + * handle them correctly when extracting, so it appears this behavior + * may not be intentional. * - * (GSHK handles empty data and resource forks by not storing a - * thread at all. It doesn't correctly deal with them when extracting - * though, so it appears this behavior wasn't entirely expected.) + * We need to create an empty file for whichever forks are missing. + * Could be the data fork, resource fork, or both. The only way to + * know what's expected is to examine the file's storage type. * - * If it's a forked file, we also need to create an empty rsrc file. - * - * If valMaskDataless is enabled, this won't fire, because we "forge" - * appropriate threads. + * If valMaskDataless is enabled, this won't fire, because we will have + * "forged" the appropriate threads. * * Note there's another one of these below, in Nu_StreamExtract. */ - if (/*pArchive->valMaskDataless &&*/ !hasInterestingThread) { - err = Nu_FakeZeroExtract(pArchive, pRecord, 0x0000 /*data*/); + Assert(!pArchive->valMaskDataless || (!needFakeData && !needFakeRsrc)); + if (needFakeData) { + err = Nu_FakeZeroExtract(pArchive, pRecord, kNuThreadKindDataFork); + BailError(err); + } + if (needFakeRsrc) { + err = Nu_FakeZeroExtract(pArchive, pRecord, kNuThreadKindRsrcFork); BailError(err); - if (pRecord->recStorageType == kNuStorageExtended) { - err = Nu_FakeZeroExtract(pArchive, pRecord, 0x0002 /*rsrc*/); - BailError(err); - } } bail: diff --git a/nufxlib/Thread.c b/nufxlib/Thread.c index d8e2683..0ad4ba0 100644 --- a/nufxlib/Thread.c +++ b/nufxlib/Thread.c @@ -193,7 +193,10 @@ NuError Nu_ReadThreadHeaders(NuArchive* pArchive, NuRecord* pRecord, NuError err = kNuErrNone; NuThread* pThread; long count; - Boolean hasData = false; + Boolean needFakeData, needFakeRsrc; + + needFakeData = true; + needFakeRsrc = (pRecord->recStorageType == kNuStorageExtended); Assert(pArchive != NULL); Assert(pRecord != NULL); @@ -215,8 +218,13 @@ NuError Nu_ReadThreadHeaders(NuArchive* pArchive, NuRecord* pRecord, err = Nu_ReadThreadHeader(pArchive, pThread, pCrc); BailError(err); - if (pThread->thThreadClass == kNuThreadClassData) - hasData = true; + if (pThread->thThreadClass == kNuThreadClassData) { + if (pThread->thThreadKind == kNuThreadKindDataFork) { + needFakeData = false; + } else if (pThread->thThreadKind == kNuThreadKindRsrcFork) { + needFakeRsrc = false; + } + } /* * Some versions of ShrinkIt write an invalid thThreadEOF for disks, @@ -258,13 +266,14 @@ NuError Nu_ReadThreadHeaders(NuArchive* pArchive, NuRecord* pRecord, * If "mask threadless" is set, create "fake" threads with empty * data and resource forks as needed. */ - if (!hasData && pArchive->valMaskDataless) { - Boolean needRsrc = (pRecord->recStorageType == kNuStorageExtended); + if ((needFakeData || needFakeRsrc) && pArchive->valMaskDataless) { int firstNewThread = pRecord->recTotalThreads; - pRecord->recTotalThreads++; - pRecord->fakeThreads++; - if (needRsrc) { + if (needFakeData) { + pRecord->recTotalThreads++; + pRecord->fakeThreads++; + } + if (needFakeRsrc) { pRecord->recTotalThreads++; pRecord->fakeThreads++; } @@ -275,22 +284,23 @@ NuError Nu_ReadThreadHeaders(NuArchive* pArchive, NuRecord* pRecord, pThread = pRecord->pThreads + firstNewThread; - pThread->thThreadClass = kNuThreadClassData; - pThread->thThreadFormat = kNuThreadFormatUncompressed; - pThread->thThreadKind = 0x0000; /* data fork */ - pThread->thThreadCRC = kNuInitialThreadCRC; - pThread->thThreadEOF = 0; - pThread->thCompThreadEOF = 0; - pThread->threadIdx = Nu_GetNextThreadIdx(pArchive); - pThread->actualThreadEOF = 0; - pThread->fileOffset = -99999999; - pThread->used = false; - - if (needRsrc) { - pThread++; + if (needFakeData) { pThread->thThreadClass = kNuThreadClassData; pThread->thThreadFormat = kNuThreadFormatUncompressed; - pThread->thThreadKind = 0x0002; /* rsrc fork */ + pThread->thThreadKind = kNuThreadKindDataFork; + pThread->thThreadCRC = kNuInitialThreadCRC; + pThread->thThreadEOF = 0; + pThread->thCompThreadEOF = 0; + pThread->threadIdx = Nu_GetNextThreadIdx(pArchive); + pThread->actualThreadEOF = 0; + pThread->fileOffset = -99999999; + pThread->used = false; + pThread++; + } + if (needFakeRsrc) { + pThread->thThreadClass = kNuThreadClassData; + pThread->thThreadFormat = kNuThreadFormatUncompressed; + pThread->thThreadKind = kNuThreadKindRsrcFork; pThread->thThreadCRC = kNuInitialThreadCRC; pThread->thThreadEOF = 0; pThread->thCompThreadEOF = 0; diff --git a/nufxlib/samples/TestBasic.c b/nufxlib/samples/TestBasic.c index 2559608..19fea41 100644 --- a/nufxlib/samples/TestBasic.c +++ b/nufxlib/samples/TestBasic.c @@ -298,7 +298,8 @@ int Test_AddStuff(NuArchive* pArchive) /* - * Create an empty file with a rather non-empty name. + * Create an empty file with a rather non-empty name. Add it as + * a resource fork. */ printf("... add 'long' record\n"); err = NuCreateDataSourceForBuffer(kNuThreadFormatUncompressed, @@ -454,7 +455,8 @@ failed: /* - * Selection callback filter for "test". This gets called once per record. + * Selection callback filter for "test". This gets called once per record, + * twice per record for forked files. */ NuResult VerifySelectionCallback(NuArchive* pArchive, void* vpProposal) { @@ -523,7 +525,9 @@ int Test_Verify(NuArchive* pArchive) goto failed; } - if (count != kNumEntries) { + /* the count will be one higher than the number of records because + the last entry is forked, and each fork is tested separately */ + if (count != kNumEntries + 1) { fprintf(stderr, "ERROR: verified %ld when expecting %d\n", count, kNumEntries); goto failed; diff --git a/nulib2/ChangeLog.txt b/nulib2/ChangeLog.txt index dfe0324..6b56bee 100644 --- a/nulib2/ChangeLog.txt +++ b/nulib2/ChangeLog.txt @@ -1,4 +1,7 @@ -2005/01/09 ***** v3.0.0 shipped ***** +2015/12/26 fadden + - Fix handling of entries with missing threads. + +2015/01/09 ***** v3.0.0 shipped ***** 2015/01/03 fadden - Mac OS X: get file/aux type from FinderInfo when adding files. diff --git a/nulib2/List.c b/nulib2/List.c index 0511b43..7e59ce7 100644 --- a/nulib2/List.c +++ b/nulib2/List.c @@ -135,10 +135,14 @@ bail: * a disk image thread, but it's a fair bet ShrinkIt would ignore one * or the other. * - * NOTE: we don't currently work around the GSHK zero-length file bug. - * Such records, which have a filename thread but no data threads at all, - * will be categorized as "unknown". We could detect the situation and - * correct it, but we might as well flag it in a user-visible way. + * NOTE: GSHK likes to omit the data threads for zero-length data and + * resource forks. That screws up analysis by scanning for threads. We + * can work around missing resource forks by simply checking the record's + * storage type. We could be clever and detect records that have no + * data-class threads at all, and no additional threads other than a + * comment and filename, but this is just for display. ShrinkIt doesn't + * handle these records correctly in all cases, so flagging them in a + * user-visible way seems reasonable. */ static NuError AnalyzeRecord(const NuRecord* pRecord, enum RecordKind* pRecordKind, uint16_t* pFormat, uint32_t* pTotalLen, @@ -177,6 +181,16 @@ static NuError AnalyzeRecord(const NuRecord* pRecord, } } + /* + * Fix up the case where we have a forked file, but GSHK decided not + * to include a resource fork in the record. + */ + if (pRecord->recStorageType == kNuStorageExtended && + *pRecordKind != kRecordKindForkedFile) + { + *pRecordKind = kRecordKindForkedFile; + } + return kNuErrNone; } diff --git a/nulib2/Main.c b/nulib2/Main.c index 0955081..16eec34 100644 --- a/nulib2/Main.c +++ b/nulib2/Main.c @@ -143,7 +143,7 @@ static void Usage(const NulibState* pState) printf("\nNuLib2 v%s, linked with NufxLib v%d.%d.%d [%s]\n", NState_GetProgramVersion(pState), majorVersion, minorVersion, bugVersion, nufxLibFlags); - printf("Copyright (C) 2000-2014, Andy McFadden. All Rights Reserved.\n"); + printf("Copyright (C) 2000-2015, Andy McFadden. All Rights Reserved.\n"); printf("This software is distributed under terms of the BSD License.\n"); printf("Visit http://www.nulib.com/ for source code and documentation.\n\n"); printf("Usage: %s -command[modifiers] archive [filename-list]\n\n", diff --git a/nulib2/State.c b/nulib2/State.c index a64bf2f..29a584b 100644 --- a/nulib2/State.c +++ b/nulib2/State.c @@ -9,7 +9,7 @@ #include "NuLib2.h" -static const char* gProgramVersion = "3.0.0"; +static const char* gProgramVersion = "3.1.0-a1"; /*