Simplify code with macros, update comments and fix whitespace.

Hopefully all of these changes are inconsequential.

Merged back changes from the test refactor branch to reduce diffs.

30 files changed, 307 insertions, 318 deletions

diff --git a/bin/bbackupd/BackupClientContext.cpp b/bin/bbackupd/BackupClientContext.cpp
index 5025bf5c..977f7ec1 100644
--- a/bin/bbackupd/BackupClientContext.cpp
+++ b/bin/bbackupd/BackupClientContext.cpp
@@ -42,11 +42,11 @@
 // --------------------------------------------------------------------------
 BackupClientContext::BackupClientContext
 (
-	LocationResolver &rResolver, 
-	TLSContext &rTLSContext, 
+	LocationResolver &rResolver,
+	TLSContext &rTLSContext,
 	const std::string &rHostname,
 	int Port,
-	uint32_t AccountNumber, 
+	uint32_t AccountNumber,
 	bool ExtendedLogging,
 	bool ExtendedLogToFile,
 	std::string ExtendedLogFile,
@@ -152,7 +152,7 @@ BackupProtocolClient &BackupClientContext::GetConnection()
 		if (mExtendedLogToFile)
 		{
 			ASSERT(mpExtendedLogFileHandle == NULL);
-			
+
 			mpExtendedLogFileHandle = fopen(
 				mExtendedLogFile.c_str(), "a+");
 
@@ -198,7 +198,7 @@ BackupProtocolClient &BackupClientContext::GetConnection()
 				{
 					// IGNORE
 				}
-				
+
 				// Then throw an exception about this
 				THROW_EXCEPTION_MESSAGE(BackupStoreException,
 					ClientMarkerNotAsExpected,
diff --git a/bin/bbackupd/BackupClientContext.h b/bin/bbackupd/BackupClientContext.h
index 7e081e2d..a1b5d824 100644
--- a/bin/bbackupd/BackupClientContext.h
+++ b/bin/bbackupd/BackupClientContext.h
@@ -42,11 +42,11 @@ class BackupClientContext : public DiffTimer
 public:
 	BackupClientContext
 	(
-		LocationResolver &rResolver, 
-		TLSContext &rTLSContext, 
+		LocationResolver &rResolver,
+		TLSContext &rTLSContext,
 		const std::string &rHostname,
 		int32_t Port,
-		uint32_t AccountNumber, 
+		uint32_t AccountNumber,
 		bool ExtendedLogging,
 		bool ExtendedLogToFile,
 		std::string ExtendedLogFile,
@@ -59,11 +59,8 @@ private:
 public:
 
 	BackupProtocolClient &GetConnection();
-	
 	void CloseAnyOpenConnection();
-	
 	int GetTimeout() const;
-	
 	BackupClientDeleteList &GetDeleteList();
 	void PerformDeletions();
 
@@ -74,7 +71,7 @@ public:
 
 	void SetClientStoreMarker(int64_t ClientStoreMarker) {mClientStoreMarker = ClientStoreMarker;}
 	int64_t GetClientStoreMarker() const {return mClientStoreMarker;}
-	
+
 	bool StorageLimitExceeded() {return mStorageLimitExceeded;}
 	void SetStorageLimitExceeded() {mStorageLimitExceeded = true;}
 
diff --git a/bin/bbackupd/BackupClientDirectoryRecord.cpp b/bin/bbackupd/BackupClientDirectoryRecord.cpp
index 9fa745ec..292330ab 100644
--- a/bin/bbackupd/BackupClientDirectoryRecord.cpp
+++ b/bin/bbackupd/BackupClientDirectoryRecord.cpp
@@ -652,14 +652,16 @@ BackupClientDirectoryRecord::FetchDirectoryListing(BackupClientDirectoryRecord::
 	std::auto_ptr<BackupStoreDirectory> apDir;
 	
 	// Get connection to store
-	BackupProtocolClient &connection(rParams.mrContext.GetConnection());
+	BackupProtocolCallable &connection(rParams.mrContext.GetConnection());
 
 	// Query the directory
 	std::auto_ptr<BackupProtocolSuccess> dirreply(connection.QueryListDirectory(
 			mObjectID,
-			BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,	// both files and directories
-			BackupProtocolListDirectory::Flags_Deleted | 
-			BackupProtocolListDirectory::Flags_OldVersion, // exclude old/deleted stuff
+			// both files and directories
+			BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
+			// exclude old/deleted stuff
+			BackupProtocolListDirectory::Flags_Deleted |
+			BackupProtocolListDirectory::Flags_OldVersion,
 			true /* want attributes */));
 
 	// Retrieve the directory from the stream following
@@ -825,7 +827,7 @@ bool BackupClientDirectoryRecord::UpdateItems(
 				rNotifier.NotifyFileStatFailed(this, nonVssFilePath,
 					strerror(errno));
 
-				// Report the error (logs and 
+				// Report the error (logs and
 				// eventual email to administrator)
 				SetErrorWhenReadingFilesystemObject(rParams, nonVssFilePath);
 
@@ -959,12 +961,12 @@ bool BackupClientDirectoryRecord::UpdateItems(
 		// Condition for upload:
 		//    modification time within sync period
 		//    if it's been seen before but not uploaded, is the time from this first sight longer than the MaxUploadWait
-		//	  and if we know about it from a directory listing, that it hasn't got the same upload time as on the store
+		//    and if we know about it from a directory listing, that it hasn't got the same upload time as on the store
 
 		bool doUpload = false;
 		std::string decisionReason = "unknown";
 
-		// Only upload a file if the mod time locally is 
+		// Only upload a file if the mod time locally is
 		// different to that on the server.
 
 		if (en == 0 || en->GetModificationTime() != modTime)
@@ -987,32 +989,32 @@ bool BackupClientDirectoryRecord::UpdateItems(
 				}
 			}
 
-			// However, just in case things are continually 
+			// However, just in case things are continually
 			// modified, we check the first seen time.
-			// The two compares of syncPeriodEnd and 
-			// pendingFirstSeenTime are because the values 
+			// The two compares of syncPeriodEnd and
+			// pendingFirstSeenTime are because the values
 			// are unsigned.
 
-			if (!doUpload && 
+			if (!doUpload &&
 				pendingFirstSeenTime != 0 &&
 				rParams.mSyncPeriodEnd > pendingFirstSeenTime &&
-				(rParams.mSyncPeriodEnd - pendingFirstSeenTime) 
+				(rParams.mSyncPeriodEnd - pendingFirstSeenTime)
 				> rParams.mMaxUploadWait)
 			{
 				doUpload = true;
 				decisionReason = "continually modified";
 			}
 
-			// Then make sure that if files are added with a 
+			// Then make sure that if files are added with a
 			// time less than the sync period start
-			// (which can easily happen on file server), it 
+			// (which can easily happen on file server), it
 			// gets uploaded. The directory contents checksum
-			// will pick up the fact it has been added, so the 
+			// will pick up the fact it has been added, so the
 			// store listing will be available when this happens.
 
 			if (!doUpload &&
-				modTime <= rParams.mSyncPeriodStart && 
-				en != 0 && 
+				modTime <= rParams.mSyncPeriodStart &&
+				en != 0 &&
 				en->GetModificationTime() != modTime)
 			{
 				doUpload = true;
@@ -1023,7 +1025,7 @@ bool BackupClientDirectoryRecord::UpdateItems(
 			// the future for file server clients,
 			// just upload the file if it's madly in the future.
 
-			if (!doUpload && modTime > 
+			if (!doUpload && modTime >
 				rParams.mUploadAfterThisTimeInTheFuture)
 			{
 				doUpload = true;
@@ -1044,14 +1046,14 @@ bool BackupClientDirectoryRecord::UpdateItems(
 				int age = BoxTimeToSeconds(now -
 					modTime);
 				std::ostringstream s;
-				s << "modified too recently: "
-					"only " << age << " seconds ago";
+				s << "modified too recently: only " <<
+					age << " seconds ago";
 				decisionReason = s.str();
 			}
 			else
 			{
 				std::ostringstream s;
-				s << "mod time is " << modTime << 
+				s << "mod time is " << modTime <<
 					" which is outside sync window, "
 					<< rParams.mSyncPeriodStart << " to "
 					<< rParams.mSyncPeriodEnd;
@@ -1082,7 +1084,7 @@ bool BackupClientDirectoryRecord::UpdateItems(
 			{
 				// Upload the file to the server, recording the
 				// object ID it returns
-				bool noPreviousVersionOnServer = 
+				bool noPreviousVersionOnServer =
 					((pDirOnStore != 0) && (en == 0));
 				
 				// Surround this in a try/catch block, to
@@ -1126,7 +1128,7 @@ bool BackupClientDirectoryRecord::UpdateItems(
 					if (e.GetType() == BackupStoreException::ExceptionType &&
 						e.GetSubType() == BackupStoreException::SignalReceived)
 					{
-						// abort requested, pass the 
+						// abort requested, pass the
 						// exception on up.
 						throw;
 					}
@@ -1287,7 +1289,7 @@ bool BackupClientDirectoryRecord::UpdateItems(
 	// Erase contents of files to save space when recursing
 	rFiles.clear();
 
-	// Delete the pending entries, if the map is entry
+	// Delete the pending entries, if the map is empty
 	if(mpPendingEntries != 0 && mpPendingEntries->size() == 0)
 	{
 		BOX_TRACE("Deleting mpPendingEntries from dir ID " <<
@@ -1761,7 +1763,7 @@ int64_t BackupClientDirectoryRecord::UploadFile(
 					rLocalPath,
 					mObjectID, /* containing directory */
 					rStoreFilename, diffFromID, *blockIndexStream,
-					connection.GetTimeout(), 
+					connection.GetTimeout(),
 					&rContext, // DiffTimer implementation
 					0 /* not interested in the modification time */, 
 					&isCompletelyDifferent,
@@ -1809,11 +1811,9 @@ int64_t BackupClientDirectoryRecord::UploadFile(
 
 		// Send to store
 		std::auto_ptr<BackupProtocolSuccess> stored(
-			connection.QueryStoreFile(
-				mObjectID, ModificationTime,
-				AttributesHash, 
-				diffFromID,
-				rStoreFilename, apWrappedStream));
+			connection.QueryStoreFile(mObjectID, ModificationTime,
+				AttributesHash, diffFromID, rStoreFilename,
+				apWrappedStream));
 
 		rContext.SetNiceMode(false);
 
diff --git a/bin/bbackupd/BackupDaemon.cpp b/bin/bbackupd/BackupDaemon.cpp
index 0fca634c..5b965c26 100644
--- a/bin/bbackupd/BackupDaemon.cpp
+++ b/bin/bbackupd/BackupDaemon.cpp
@@ -870,8 +870,8 @@ void BackupDaemon::RunSyncNow()
 	// just connect, as this may be unnecessary)
 	BackupClientContext clientContext
 	(
-		*mpLocationResolver, 
-		mTlsContext, 
+		*mpLocationResolver,
+		mTlsContext,
 		conf.GetKeyValue("StoreHostname"),
 		conf.GetKeyValueInt("StorePort"),
 		conf.GetKeyValueUint32("AccountNumber"),
@@ -929,19 +929,18 @@ void BackupDaemon::RunSyncNow()
 	// Paranoid check on sync times
 	if(syncPeriodStart >= syncPeriodEnd) return;
 	
-	// Adjust syncPeriodEnd to emulate snapshot 
-	// behaviour properly
+	// Adjust syncPeriodEnd to emulate snapshot behaviour properly
 	box_time_t syncPeriodEndExtended = syncPeriodEnd;
 
 	// Using zero min file age?
 	if(minimumFileAge == 0)
 	{
 		// Add a year on to the end of the end time,
-		// to make sure we sync files which are 
+		// to make sure we sync files which are
 		// modified after the scan run started.
-		// Of course, they may be eligible to be 
+		// Of course, they may be eligible to be
 		// synced again the next time round,
-		// but this should be OK, because the changes 
+		// but this should be OK, because the changes
 		// only upload should upload no data.
 		syncPeriodEndExtended += SecondsToBoxTime(
 			(time_t)(356*24*3600));
@@ -954,11 +953,11 @@ void BackupDaemon::RunSyncNow()
 	params.mSyncPeriodEnd = syncPeriodEndExtended;
 	// use potentially extended end time
 	params.mMaxUploadWait = maxUploadWait;
-	params.mFileTrackingSizeThreshold = 
+	params.mFileTrackingSizeThreshold =
 		conf.GetKeyValueInt("FileTrackingSizeThreshold");
-	params.mDiffingUploadSizeThreshold = 
+	params.mDiffingUploadSizeThreshold =
 		conf.GetKeyValueInt("DiffingUploadSizeThreshold");
-	params.mMaxFileTimeInFuture = 
+	params.mMaxFileTimeInFuture =
 		SecondsToBoxTime(conf.GetKeyValueInt("MaxFileTimeInFuture"));
 	mNumFilesUploaded = 0;
 	mNumDirsCreated = 0;
@@ -997,22 +996,21 @@ void BackupDaemon::RunSyncNow()
 	
 	// Set store marker
 	clientContext.SetClientStoreMarker(mClientStoreMarker);
-	
-	// Set up the locations, if necessary -- 
-	// need to do it here so we have a 
-	// (potential) connection to use
+
+	// Set up the locations, if necessary -- need to do it here so we have
+	// a (potential) connection to use.
 	{
 		const Configuration &locations(
 			conf.GetSubConfiguration(
 				"BackupLocations"));
-		
+
 		// Make sure all the directory records
 		// are set up
 		SetupLocations(clientContext, locations);
 	}
-	
+
 	mpProgressNotifier->NotifyIDMapsSetup(clientContext);
-	
+
 	// Get some ID maps going
 	SetupIDMapsForSync();
 
@@ -1022,7 +1020,7 @@ void BackupDaemon::RunSyncNow()
 #ifdef ENABLE_VSS
 	CreateVssBackupComponents();
 #endif
-					
+
 	// Go through the records, syncing them
 	for(Locations::const_iterator 
 		i(mLocations.begin()); 
@@ -1055,7 +1053,7 @@ void BackupDaemon::RunSyncNow()
 		// Unset exclude lists (just in case)
 		clientContext.SetExcludeLists(0, 0);
 	}
-	
+
 	// Perform any deletions required -- these are
 	// delayed until the end to allow renaming to 
 	// happen neatly.
@@ -1087,8 +1085,8 @@ void BackupDaemon::RunSyncNow()
 	CommitIDMapsAfterSync();
 
 	// Calculate when the next sync run should be
-	mNextSyncTime = mCurrentSyncStartTime + 
-		mUpdateStoreInterval + 
+	mNextSyncTime = mCurrentSyncStartTime +
+		mUpdateStoreInterval +
 		Random::RandomInt(mUpdateStoreInterval >>
 		SYNC_PERIOD_RANDOM_EXTRA_TIME_SHIFT_BY);
 
@@ -1098,7 +1096,7 @@ void BackupDaemon::RunSyncNow()
 	// info. If we save successfully, we must 
 	// delete the file next time we start a backup
 
-	mDeleteStoreObjectInfoFile = 
+	mDeleteStoreObjectInfoFile =
 		SerializeStoreObjectInfo(mLastSyncTime, mNextSyncTime);
 
 	// --------------------------------------------------------------------------------------------
@@ -1804,7 +1802,8 @@ int BackupDaemon::ParseSyncAllowScriptOutput(const std::string& script,
 
 	if(delay == "")
 	{
-		BOX_ERROR("SyncAllowScript output an empty line");
+		BOX_ERROR("SyncAllowScript output an empty line, sleeping for "
+			<< waitInSeconds << " seconds (" << script << ")");
 		return waitInSeconds;
 	}
 
@@ -1815,7 +1814,7 @@ int BackupDaemon::ParseSyncAllowScriptOutput(const std::string& script,
 		waitInSeconds = -1;
 
 		BOX_NOTICE("SyncAllowScript requested a backup now "
-			<< "(" << script << ")");
+			"(" << script << ")");
 	}
 	else
 	{
@@ -1832,7 +1831,7 @@ int BackupDaemon::ParseSyncAllowScriptOutput(const std::string& script,
 			throw;
 		}
 
-		BOX_NOTICE("SyncAllowScript requested a delay of " << 
+		BOX_NOTICE("SyncAllowScript requested a delay of " <<
 			waitInSeconds << " seconds (" << script << ")");
 	}
 
@@ -2546,7 +2545,7 @@ void BackupDaemon::SetupLocations(BackupClientContext &rClientContext, const Con
 						new MemBlockStream(attr));
 					std::auto_ptr<BackupProtocolSuccess>
 						dirCreate(connection.QueryCreateDirectory(
-						BackupProtocolListDirectory::RootDirectory,
+						BACKUPSTORE_ROOT_DIRECTORY_ID, // containing directory
 						attrModTime, dirname, attrStream));
 						
 					// Object ID for later creation
diff --git a/bin/bbackupd/BackupDaemon.h b/bin/bbackupd/BackupDaemon.h
index ca3382fa..925aea1a 100644
--- a/bin/bbackupd/BackupDaemon.h
+++ b/bin/bbackupd/BackupDaemon.h
@@ -237,21 +237,21 @@ public:
 	void SetSysadminNotifier (SysadminNotifier*  p) { mpSysadminNotifier = p; }
 	virtual bool RunBackgroundTask(State state, uint64_t progress,
 		uint64_t maximum);
-		
+
 private:
 	ProgressNotifier* mpProgressNotifier;
 	LocationResolver* mpLocationResolver;
 	RunStatusProvider* mpRunStatusProvider;
 	SysadminNotifier* mpSysadminNotifier;
 	std::auto_ptr<Timer> mapCommandSocketPollTimer;
-	
- 	/* ProgressNotifier implementation */
+
+	/* ProgressNotifier implementation */
 public:
 	virtual void NotifyIDMapsSetup(BackupClientContext& rContext) { }
 
 	virtual void NotifyScanDirectory(
 		const BackupClientDirectoryRecord* pDirRecord,
-		const std::string& rLocalPath) 
+		const std::string& rLocalPath)
 	{
 		if (mLogAllFileAccess)
 		{
@@ -266,7 +266,7 @@ public:
 	}
 	virtual void NotifyDirStatFailed(
 		const BackupClientDirectoryRecord* pDirRecord,
-		const std::string& rLocalPath, 
+		const std::string& rLocalPath,
 		const std::string& rErrorMsg)
 	{
 		BOX_WARNING("Failed to access directory: " << rLocalPath
@@ -293,11 +293,11 @@ public:
 		const std::string& rLocalPath)
 	{
 		#ifdef WIN32
-			BOX_WARNING("Ignored directory: " << rLocalPath << 
+			BOX_WARNING("Ignored directory: " << rLocalPath <<
 				": is an NTFS junction/reparse point; create "
 				"a new location if you want to back it up");
 		#else
-			BOX_WARNING("Ignored directory: " << rLocalPath << 
+			BOX_WARNING("Ignored directory: " << rLocalPath <<
 				": is a mount point; create a new location "
 				"if you want to back it up");
 		#endif
@@ -446,7 +446,7 @@ public:
 	{
 		if (mLogAllFileAccess)
 		{
-			BOX_NOTICE("Deleted directory: " << rRemotePath << 
+			BOX_NOTICE("Deleted directory: " << rRemotePath <<
 				" (ID " << BOX_FORMAT_OBJECTID(ObjectID) <<
 				")");
 		}
@@ -457,7 +457,7 @@ public:
 	{
 		if (mLogAllFileAccess)
 		{
-			BOX_NOTICE("Deleted file: " << rRemotePath << 
+			BOX_NOTICE("Deleted file: " << rRemotePath <<
 				" (ID " << BOX_FORMAT_OBJECTID(ObjectID) <<
 				")");
 		}
@@ -465,19 +465,19 @@ public:
 	virtual void NotifyReadProgress(int64_t readSize, int64_t offset,
 		int64_t length, box_time_t elapsed, box_time_t finish)
 	{
-		BOX_TRACE("Read " << readSize << " bytes at " << offset << 
+		BOX_TRACE("Read " << readSize << " bytes at " << offset <<
 			", " << (length - offset) << " remain, eta " <<
 			BoxTimeToSeconds(finish - elapsed) << "s");
 	}
 	virtual void NotifyReadProgress(int64_t readSize, int64_t offset,
 		int64_t length)
 	{
-		BOX_TRACE("Read " << readSize << " bytes at " << offset << 
+		BOX_TRACE("Read " << readSize << " bytes at " << offset <<
 			", " << (length - offset) << " remain");
 	}
 	virtual void NotifyReadProgress(int64_t readSize, int64_t offset)
 	{
-		BOX_TRACE("Read " << readSize << " bytes at " << offset << 
+		BOX_TRACE("Read " << readSize << " bytes at " << offset <<
 			", unknown bytes remaining");
 	}
 
diff --git a/bin/bbackupd/bbackupd.cpp b/bin/bbackupd/bbackupd.cpp
index bb64f745..5977615f 100644
--- a/bin/bbackupd/bbackupd.cpp
+++ b/bin/bbackupd/bbackupd.cpp
@@ -45,10 +45,12 @@ int main(int argc, const char *argv[])
 
 #else // !WIN32
 
-	BackupDaemon daemon;
-	ExitCode = daemon.Main(BOX_GET_DEFAULT_BBACKUPD_CONFIG_FILE,
-		argc, argv);
-
+	{
+		BackupDaemon daemon;
+		ExitCode = daemon.Main(BOX_GET_DEFAULT_BBACKUPD_CONFIG_FILE,
+			argc, argv);
+	}
+	
 #endif // WIN32
 
 	MAINHELPER_END
diff --git a/bin/bbackupquery/BackupQueries.cpp b/bin/bbackupquery/BackupQueries.cpp
index c701ff7e..7bd2e367 100644
--- a/bin/bbackupquery/BackupQueries.cpp
+++ b/bin/bbackupquery/BackupQueries.cpp
@@ -291,8 +291,8 @@ void BackupQueries::CommandList(const std::vector<std::string> &args, const bool
 #endif
 	
 		// Attempt to find the directory
-		rootDir = FindDirectoryObjectID(storeDirEncoded, 
-			opts[LIST_OPTION_ALLOWOLD], 
+		rootDir = FindDirectoryObjectID(storeDirEncoded,
+			opts[LIST_OPTION_ALLOWOLD],
 			opts[LIST_OPTION_ALLOWDELETED]);
 
 		if(rootDir == 0)
diff --git a/lib/backupclient/BackupClientRestore.cpp b/lib/backupclient/BackupClientRestore.cpp
index a586dc47..d3300604 100644
--- a/lib/backupclient/BackupClientRestore.cpp
+++ b/lib/backupclient/BackupClientRestore.cpp
@@ -224,7 +224,7 @@ static int BackupClientRestoreDir(BackupProtocolCallable &rConnection,
 			DIRECTORY_SEPARATOR_ASCHAR + 
 			rLevel.mNextLevelLocalName);
 		BackupClientRestoreDir(rConnection, rLevel.mNextLevelID,
-			rRemoteDirectoryName + '/' + 
+			rRemoteDirectoryName + '/' +
 			rLevel.mNextLevelLocalName, localDirname,
 			Params, *rLevel.mpNextLevel);
 		
@@ -232,7 +232,7 @@ static int BackupClientRestoreDir(BackupProtocolCallable &rConnection,
 		rLevel.mRestoredObjects.insert(rLevel.mNextLevelID);
 
 		// Remove the level for the recursed directory
-		rLevel.RemoveLevel();		
+		rLevel.RemoveLevel();
 	}
 	
 	// Create the local directory, if not already done.
@@ -299,7 +299,7 @@ static int BackupClientRestoreDir(BackupProtocolCallable &rConnection,
 	}
 
 	std::string parentDirectoryName(rLocalDirectoryName);
-	if(parentDirectoryName[parentDirectoryName.size() - 1] == 
+	if(parentDirectoryName[parentDirectoryName.size() - 1] ==
 		DIRECTORY_SEPARATOR_ASCHAR)
 	{
 		parentDirectoryName.resize(parentDirectoryName.size() - 1);
@@ -309,7 +309,7 @@ static int BackupClientRestoreDir(BackupProtocolCallable &rConnection,
 
 	if(lastSlash == std::string::npos)
 	{
-		// might be a forward slash separator, 
+		// might be a forward slash separator,
 		// especially in the unit tests!
 		lastSlash = parentDirectoryName.rfind('/');
 	}
@@ -889,7 +889,7 @@ int BackupClientRestore(BackupProtocolCallable &rConnection,
 	}
 	
 	// Restore the directory
-	int result = BackupClientRestoreDir(rConnection, DirectoryID, 
+	int result = BackupClientRestoreDir(rConnection, DirectoryID,
 		RemoteDirectoryName, LocalDirectoryName, params,
 		params.mResumeInfo);
 	if (result != Restore_Complete)
diff --git a/lib/backupstore/BackupProtocol.h b/lib/backupstore/BackupProtocol.h
index 43f3e162..62244aae 100644
--- a/lib/backupstore/BackupProtocol.h
+++ b/lib/backupstore/BackupProtocol.h
@@ -31,7 +31,7 @@ private:
 
 public:
 	BackupProtocolLocal2(int32_t AccountNumber,
-		const std::string& ConnectionDetails, 
+		const std::string& ConnectionDetails,
 		const std::string& AccountRootDir, int DiscSetNumber,
 		bool ReadOnly)
 	// This is rather ugly: the BackupProtocolLocal constructor must not
diff --git a/lib/backupstore/BackupStoreContext.cpp b/lib/backupstore/BackupStoreContext.cpp
index 5b94cd41..d9912510 100644
--- a/lib/backupstore/BackupStoreContext.cpp
+++ b/lib/backupstore/BackupStoreContext.cpp
@@ -350,12 +350,9 @@ BackupStoreDirectory &BackupStoreContext::GetDirectoryInternal(int64_t ObjectID)
 	std::auto_ptr<RaidFileRead> objectFile(RaidFileRead::Open(mStoreDiscSet, filename, &revID));
 	ASSERT(revID != 0);
 	
-	// New directory object
-	std::auto_ptr<BackupStoreDirectory> dir(new BackupStoreDirectory);
-	
 	// Read it from the stream, then set it's revision ID
 	BufferedStream buf(*objectFile);
-	dir->ReadFromStream(buf, IOStream::TimeOutInfinite);
+	std::auto_ptr<BackupStoreDirectory> dir(new BackupStoreDirectory(buf));
 	dir->SetRevisionID(revID);
 			
 	// Make sure the size of the directory is available for writing the dir back
diff --git a/lib/backupstore/BackupStoreDirectory.cpp b/lib/backupstore/BackupStoreDirectory.cpp
index 0c575c51..edef25bc 100644
--- a/lib/backupstore/BackupStoreDirectory.cpp
+++ b/lib/backupstore/BackupStoreDirectory.cpp
@@ -480,7 +480,8 @@ void BackupStoreDirectory::Entry::ReadFromStream(IOStream &rStream, int Timeout)
 {
 	// Grab the raw bytes from the stream which compose the header
 	en_StreamFormat entry;
-	if(!rStream.ReadFullBuffer(&entry, sizeof(entry), 0 /* not interested in bytes read if this fails */, Timeout))
+	if(!rStream.ReadFullBuffer(&entry, sizeof(entry),
+		0 /* not interested in bytes read if this fails */, Timeout))
 	{
 		THROW_EXCEPTION(BackupStoreException, CouldntReadEntireStructureFromStream)
 	}
diff --git a/lib/backupstore/BackupStoreFileDiff.cpp b/lib/backupstore/BackupStoreFileDiff.cpp
index f1036354..fd186e7c 100644
--- a/lib/backupstore/BackupStoreFileDiff.cpp
+++ b/lib/backupstore/BackupStoreFileDiff.cpp
@@ -114,20 +114,19 @@ void BackupStoreFile::MoveStreamPositionToBlockIndex(IOStream &rStream)
 // Function
 //		Name:    BackupStoreFile::EncodeFileDiff(const char *, int64_t, const BackupStoreFilename &, int64_t, IOStream &, int64_t *)
 //		Purpose: Similar to EncodeFile, but takes the object ID of the file it's
-//				 diffing from, and the index of the blocks in a stream. It'll then
-//				 calculate which blocks can be reused from that old file.
-//				 The timeout is the timeout value for reading the diff block index.
-//				 If pIsCompletelyDifferent != 0, it will be set to true if the
-//				 the two files are completely different (do not share any block), false otherwise.
-//				 
+//			 diffing from, and the index of the blocks in a stream. It'll then
+//			 calculate which blocks can be reused from that old file.
+//			 The timeout is the timeout value for reading the diff block index.
+//			 If pIsCompletelyDifferent != 0, it will be set to true if the
+//			 the two files are completely different (do not share any block), false otherwise.
 //		Created: 12/1/04
 //
 // --------------------------------------------------------------------------
 std::auto_ptr<BackupStoreFileEncodeStream> BackupStoreFile::EncodeFileDiff
 (
 	const std::string& Filename, int64_t ContainerID,
-	const BackupStoreFilename &rStoreFilename, int64_t DiffFromObjectID, 
-	IOStream &rDiffFromBlockIndex, int Timeout, DiffTimer *pDiffTimer, 
+	const BackupStoreFilename &rStoreFilename, int64_t DiffFromObjectID,
+	IOStream &rDiffFromBlockIndex, int Timeout, DiffTimer *pDiffTimer,
 	int64_t *pModificationTime, bool *pIsCompletelyDifferent,
 	BackgroundTask* pBackgroundTask)
 {
@@ -528,7 +527,7 @@ static void SearchForMatchingBlocks(IOStream &rFile, std::map<int64_t, int64_t>
 		
 		// Search for each block size in turn
 		// NOTE: Do the smallest size first, so that the scheme for adding
-		// entries in the found list works as expected and replaces smallers block
+		// entries in the found list works as expected and replaces smaller blocks
 		// with larger blocks when it finds matches at the same offset in the file.
 		for(int s = BACKUP_FILE_DIFF_MAX_BLOCK_SIZES - 1; s >= 0; --s)
 		{
@@ -670,6 +669,7 @@ static void SearchForMatchingBlocks(IOStream &rFile, std::map<int64_t, int64_t>
 						}
 						else
 						{
+							// Too many to log
 							// BOX_TRACE("False alarm match of " << Sizes[s] << " bytes with hash " << hash << " at offset " << fileOffset);
 						}
 
diff --git a/lib/backupstore/BackupStoreFileEncodeStream.cpp b/lib/backupstore/BackupStoreFileEncodeStream.cpp
index ccab8074..2b7d067a 100644
--- a/lib/backupstore/BackupStoreFileEncodeStream.cpp
+++ b/lib/backupstore/BackupStoreFileEncodeStream.cpp
@@ -709,11 +709,12 @@ bool BackupStoreFileEncodeStream::StreamClosed()
 //		Created: 15/1/04
 //
 // --------------------------------------------------------------------------
-BackupStoreFileEncodeStream::Recipe::Recipe(BackupStoreFileCreation::BlocksAvailableEntry *pBlockIndex,
-		int64_t NumBlocksInIndex, int64_t OtherFileID)
-	: mpBlockIndex(pBlockIndex),
-	  mNumBlocksInIndex(NumBlocksInIndex),
-	  mOtherFileID(OtherFileID)
+BackupStoreFileEncodeStream::Recipe::Recipe(
+	BackupStoreFileCreation::BlocksAvailableEntry *pBlockIndex,
+	int64_t NumBlocksInIndex, int64_t OtherFileID)
+: mpBlockIndex(pBlockIndex),
+  mNumBlocksInIndex(NumBlocksInIndex),
+  mOtherFileID(OtherFileID)
 {
 	ASSERT((mpBlockIndex == 0) || (NumBlocksInIndex != 0))
 }
diff --git a/lib/common/BoxTime.h b/lib/common/BoxTime.h
index 3108d809..6afaada3 100644
--- a/lib/common/BoxTime.h
+++ b/lib/common/BoxTime.h
@@ -10,8 +10,8 @@
 #ifndef BOXTIME__H
 #define BOXTIME__H
 
-// Time is presented as an unsigned 64 bit integer, in microseconds
-typedef int64_t	box_time_t;
+// Time is presented as a signed 64 bit integer, in microseconds
+typedef int64_t box_time_t;
 
 #define NANO_SEC_IN_SEC		(1000000000LL)
 #define NANO_SEC_IN_USEC 	(1000)
diff --git a/lib/common/DebugMemLeakFinder.cpp b/lib/common/DebugMemLeakFinder.cpp
index 70026969..30b2f41f 100644
--- a/lib/common/DebugMemLeakFinder.cpp
+++ b/lib/common/DebugMemLeakFinder.cpp
@@ -599,9 +599,6 @@ void memleakfinder_setup_exit_report(const char *filename, const char *markertex
 	}
 }
 
-
-
-
 void add_object_block(void *block, size_t size, const char *file, int line, bool array)
 {
 	InternalAllocGuard guard;
diff --git a/lib/common/FileStream.cpp b/lib/common/FileStream.cpp
index 6d5810dc..99b66666 100644
--- a/lib/common/FileStream.cpp
+++ b/lib/common/FileStream.cpp
@@ -248,7 +248,7 @@ IOStream::pos_type FileStream::BytesLeftToRead()
 // --------------------------------------------------------------------------
 void FileStream::Write(const void *pBuffer, int NBytes, int Timeout)
 {
-	if(mOSFileHandle == INVALID_FILE) 
+	if(mOSFileHandle == INVALID_FILE)
 	{
 		THROW_EXCEPTION(CommonException, FileClosed)
 	}
diff --git a/lib/common/IOStream.h b/lib/common/IOStream.h
index 80915d71..65b66d71 100644
--- a/lib/common/IOStream.h
+++ b/lib/common/IOStream.h
@@ -54,7 +54,7 @@ public:
 	virtual pos_type GetPosition() const;
 	virtual void Seek(pos_type Offset, int SeekType);
 	virtual void Close();
-	
+
 	// Has all data that can be read been read?
 	virtual bool StreamDataLeft() = 0;
 	// Has the stream been closed (writing not possible)
@@ -69,7 +69,4 @@ public:
 	virtual std::string ToString() const;
 };
 
-
 #endif // IOSTREAM__H
-
-
diff --git a/lib/common/ReadLoggingStream.h b/lib/common/ReadLoggingStream.h
index 4a01a45c..bee7e1d6 100644
--- a/lib/common/ReadLoggingStream.h
+++ b/lib/common/ReadLoggingStream.h
@@ -49,7 +49,7 @@ public:
 	virtual bool StreamClosed();
 
 private:
-	ReadLoggingStream(const ReadLoggingStream &rToCopy) 
+	ReadLoggingStream(const ReadLoggingStream &rToCopy)
 	: mrSource(rToCopy.mrSource), mrLogger(rToCopy.mrLogger)
 	{ /* do not call */ }
 };
diff --git a/lib/common/Test.cpp b/lib/common/Test.cpp
index bfa7bcb0..67c27cf3 100644
--- a/lib/common/Test.cpp
+++ b/lib/common/Test.cpp
@@ -136,7 +136,7 @@ int ReadPidFile(const char *pidFile)
 	if(!TestFileNotEmpty(pidFile))
 	{
 		TEST_FAIL_WITH_MESSAGE("Server didn't save PID file "
-			"(perhaps one was already running?)");	
+			"(perhaps one was already running?)");
 		return -1;
 	}
 	
@@ -145,7 +145,7 @@ int ReadPidFile(const char *pidFile)
 	FILE *f = fopen(pidFile, "r");
 	if(f == NULL || fscanf(f, "%d", &pid) != 1)
 	{
-		TEST_FAIL_WITH_MESSAGE("Couldn't read PID file");	
+		TEST_FAIL_WITH_MESSAGE("Couldn't read PID file");
 		return -1;
 	}
 	fclose(f);
@@ -155,7 +155,7 @@ int ReadPidFile(const char *pidFile)
 
 int LaunchServer(const std::string& rCommandLine, const char *pidFile)
 {
-	::fprintf(stdout, "Starting server: %s\n", rCommandLine.c_str());
+	BOX_INFO("Starting server: " << rCommandLine);
 
 #ifdef WIN32
 
@@ -189,14 +189,10 @@ int LaunchServer(const std::string& rCommandLine, const char *pidFile)
 
 	free(tempCmd);
 
-	if (result == 0)
-	{
-		DWORD err = GetLastError();
-		printf("Launch failed: %s: error %d\n", rCommandLine.c_str(),
-			(int)err);
-		TEST_FAIL_WITH_MESSAGE("Couldn't start server");
+	TEST_THAT_OR(result != 0,
+		BOX_LOG_WIN_ERROR("Launch failed: " << rCommandLine);
 		return -1;
-	}
+		);
 
 	CloseHandle(procInfo.hProcess);
 	CloseHandle(procInfo.hThread);
@@ -205,11 +201,10 @@ int LaunchServer(const std::string& rCommandLine, const char *pidFile)
 
 #else // !WIN32
 
-	if(RunCommand(rCommandLine) != 0)
-	{
-		TEST_FAIL_WITH_MESSAGE("Couldn't start server");
+	TEST_THAT_OR(RunCommand(rCommandLine) == 0,
+		TEST_FAIL_WITH_MESSAGE("Failed to start server: " << rCommandLine);
 		return -1;
-	}
+		)
 
 	return WaitForServerStartup(pidFile, 0);
 
@@ -234,7 +229,7 @@ int WaitForServerStartup(const char *pidFile, int pidIfKnown)
 
 	for (int i = 0; i < 15; i++)
 	{
-		if (TestFileNotEmpty(pidFile))	
+		if (TestFileNotEmpty(pidFile))
 		{
 			break;
 		}
@@ -252,13 +247,13 @@ int WaitForServerStartup(const char *pidFile, int pidIfKnown)
 
 	if (pidIfKnown && !ServerIsAlive(pidIfKnown))
 	{
-		TEST_FAIL_WITH_MESSAGE("Server died!");	
+		TEST_FAIL_WITH_MESSAGE("Server died!");
 		return -1;
 	}
 
 	if (!TestFileNotEmpty(pidFile))
 	{
-		TEST_FAIL_WITH_MESSAGE("Server didn't save PID file");	
+		TEST_FAIL_WITH_MESSAGE("Server didn't save PID file");
 		return -1;
 	}
 
@@ -381,7 +376,7 @@ void terminate_bbackupd(int pid)
 // Wait a given number of seconds for something to complete
 void wait_for_operation(int seconds, const char* message)
 {
-	BOX_TRACE("Waiting " << seconds << " seconds for " << message);
+	BOX_INFO("Waiting " << seconds << " seconds for " << message);
 
 	for(int l = 0; l < seconds; ++l)
 	{
@@ -395,4 +390,3 @@ void safe_sleep(int seconds)
 {
 	ShortSleep(SecondsToBoxTime(seconds), true);
 }
-
diff --git a/lib/common/Test.h b/lib/common/Test.h
index dbe1e979..146fafae 100644
--- a/lib/common/Test.h
+++ b/lib/common/Test.h
@@ -157,6 +157,9 @@ extern std::list<std::string> run_only_named_tests;
 		printf("Test failed on <%s>\n", _line_str.c_str()); \
 	}
 
+#define TEST_STARTSWITH(expected, actual) \
+	TEST_EQUAL_LINE(expected, actual.substr(0, std::string(expected).size()), actual);
+
 bool TestFileExists(const char *Filename);
 bool TestDirExists(const char *Filename);
 
diff --git a/lib/common/Timer.cpp b/lib/common/Timer.cpp
index ad6b5e8d..68042db2 100644
--- a/lib/common/Timer.cpp
+++ b/lib/common/Timer.cpp
@@ -155,7 +155,7 @@ void Timers::Remove(Timer& rTimer)
 			}
 		}
 	}
-		
+
 	Reschedule();
 }
 
diff --git a/lib/intercept/intercept.cpp b/lib/intercept/intercept.cpp
index 7a33b610..ee79fe00 100644
--- a/lib/intercept/intercept.cpp
+++ b/lib/intercept/intercept.cpp
@@ -132,7 +132,7 @@ void intercept_setup_error(const char *filename, unsigned int errorafter, int er
 	intercept_delay_ms = 0;
 }
 
-void intercept_setup_delay(const char *filename, unsigned int delay_after, 
+void intercept_setup_delay(const char *filename, unsigned int delay_after,
 	int delay_ms, int syscall_to_delay, int num_delays)
 {
 	BOX_TRACE("Setup for delay: " << filename <<
diff --git a/lib/raidfile/RaidFileUtil.cpp b/lib/raidfile/RaidFileUtil.cpp
index 7c6299ec..e826e7d0 100644
--- a/lib/raidfile/RaidFileUtil.cpp
+++ b/lib/raidfile/RaidFileUtil.cpp
@@ -14,7 +14,7 @@
 
 #include "RaidFileUtil.h"
 #include "FileModificationTime.h"
-#include "RaidFileRead.h"	// for type definition
+#include "RaidFileRead.h" // for type definition
 
 #include "MemLeakFindOn.h"
 
diff --git a/lib/server/ServerTLS.h b/lib/server/ServerTLS.h
index 20e55964..f748f4b2 100644
--- a/lib/server/ServerTLS.h
+++ b/lib/server/ServerTLS.h
@@ -52,7 +52,8 @@ public:
 		std::string certFile(serverconf.GetKeyValue("CertificateFile"));
 		std::string keyFile(serverconf.GetKeyValue("PrivateKeyFile"));
 		std::string caFile(serverconf.GetKeyValue("TrustedCAsFile"));
-		mContext.Initialise(true /* as server */, certFile.c_str(), keyFile.c_str(), caFile.c_str());
+		mContext.Initialise(true /* as server */, certFile.c_str(),
+			keyFile.c_str(), caFile.c_str());
 	
 		// Then do normal stream server stuff
 		ServerStream<SocketStreamTLS, Port, ListenBacklog,
diff --git a/lib/server/SocketStream.cpp b/lib/server/SocketStream.cpp
index db71227f..ab0a54ae 100644
--- a/lib/server/SocketStream.cpp
+++ b/lib/server/SocketStream.cpp
@@ -323,15 +323,15 @@ bool SocketStream::Poll(short Events, int Timeout)
 // --------------------------------------------------------------------------
 void SocketStream::Write(const void *pBuffer, int NBytes, int Timeout)
 {
-	if(mSocketHandle == INVALID_SOCKET_VALUE) 
+	if(mSocketHandle == INVALID_SOCKET_VALUE)
 	{
 		THROW_EXCEPTION(ServerException, BadSocketHandle)
 	}
-	
+
 	// Buffer in byte sized type.
 	ASSERT(sizeof(char) == 1);
 	const char *buffer = (char *)pBuffer;
-	
+
 	// Bytes left to send
 	int bytesLeft = NBytes;
 	box_time_t start = GetCurrentBoxTime();
@@ -348,22 +348,21 @@ void SocketStream::Write(const void *pBuffer, int NBytes, int Timeout)
 		{
 			// Error.
 			mWriteClosed = true;	// assume can't write again
-			BOX_LOG_SYS_ERROR("Failed to write to socket");
-			THROW_EXCEPTION(ConnectionException,
-				SocketWriteError);
+			THROW_SYS_ERROR("Failed to write to socket",
+				ConnectionException, SocketWriteError);
 		}
-		
+
 		// Knock off bytes sent
 		bytesLeft -= sent;
 		// Move buffer pointer
 		buffer += sent;
 
 		mBytesWritten += sent;
-		
+
 		// Need to wait until it can send again?
 		if(bytesLeft > 0)
 		{
-			BOX_TRACE("Waiting to send data on socket " << 
+			BOX_TRACE("Waiting to send data on socket " <<
 				mSocketHandle << " (" << bytesLeft <<
 				" of " << NBytes << " bytes left)");
 
@@ -388,7 +387,7 @@ void SocketStream::Write(const void *pBuffer, int NBytes, int Timeout)
 // --------------------------------------------------------------------------
 void SocketStream::Close()
 {
-	if(mSocketHandle == INVALID_SOCKET_VALUE) 
+	if(mSocketHandle == INVALID_SOCKET_VALUE)
 	{
 		THROW_EXCEPTION(ServerException, BadSocketHandle)
 	}
@@ -419,14 +418,14 @@ void SocketStream::Shutdown(bool Read, bool Write)
 	{
 		THROW_EXCEPTION(ServerException, BadSocketHandle)
 	}
-	
+
 	// Do anything?
 	if(!Read && !Write) return;
-	
+
 	int how = SHUT_RDWR;
 	if(Read && !Write) how = SHUT_RD;
 	if(!Read && Write) how = SHUT_WR;
-	
+
 	// Shut it down!
 	if(::shutdown(mSocketHandle, how) == -1)
 	{
diff --git a/lib/server/SocketStreamTLS.cpp b/lib/server/SocketStreamTLS.cpp
index e31ac13f..953012d1 100644
--- a/lib/server/SocketStreamTLS.cpp
+++ b/lib/server/SocketStreamTLS.cpp
@@ -132,7 +132,7 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer)
 
 	tOSSocketHandle socket = GetSocketHandle();
 	BIO_set_fd(mpBIO, socket, BIO_NOCLOSE);
-	
+
 	// Then the SSL object
 	mpSSL = ::SSL_new(rContext.GetRawContext());
 	if(mpSSL == 0)
@@ -155,7 +155,7 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer)
 		THROW_EXCEPTION(ServerException, SocketSetNonBlockingFailed)
 	}
 #endif
-	
+
 	// FIXME: This is less portable than the above. However, it MAY be needed
 	// for cygwin, which has/had bugs with fcntl
 	//
@@ -223,8 +223,9 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer)
 //
 // Function
 //		Name:    WaitWhenRetryRequired(int, int)
-//		Purpose: Waits until the condition required by the TLS layer is met.
-//				 Returns true if the condition is met, false if timed out.
+//		Purpose: Waits until the condition required by the TLS layer
+//		         is met. Returns true if the condition is met, false
+//		         if timed out.
 //		Created: 2003/08/15
 //
 // --------------------------------------------------------------------------
@@ -320,20 +321,20 @@ int SocketStreamTLS::Read(void *pBuffer, int NBytes, int Timeout)
 void SocketStreamTLS::Write(const void *pBuffer, int NBytes, int Timeout)
 {
 	if(!mpSSL) {THROW_EXCEPTION(ServerException, TLSNoSSLObject)}
-	
+
 	// Make sure zero byte writes work as expected
 	if(NBytes == 0)
 	{
 		return;
 	}
-	
+
 	// from man SSL_write
 	//
 	// SSL_write() will only return with success, when the
 	// complete contents of buf of length num has been written.
 	//
 	// So no worries about partial writes and moving the buffer around
-	
+
 	while(true)
 	{
 		// try the write
diff --git a/lib/server/makeprotocol.pl.in b/lib/server/makeprotocol.pl.in
index 7dfab7b3..ed0f1ee6 100755
--- a/lib/server/makeprotocol.pl.in
+++ b/lib/server/makeprotocol.pl.in
@@ -811,10 +811,12 @@ foreach my $type ('Client', 'Server', 'Local')
 	{
 		push @base_classes, $replyable_base_class;
 	}
+
 	if (not $writing_server)
 	{
 		push @base_classes, $callable_base_class;
 	}
+
 	if (not $writing_local)
 	{
 		push @base_classes, $custom_protocol_subclass;
diff --git a/test/backupstore/testbackupstore.cpp b/test/backupstore/testbackupstore.cpp
index 3bdf0102..5792fec7 100644
--- a/test/backupstore/testbackupstore.cpp
+++ b/test/backupstore/testbackupstore.cpp
@@ -409,15 +409,14 @@ bool test_backupstore_directory()
 		}
 	}
 
-	tearDown();
-	return true;
+	return tearDown();
 }
 
 void write_test_file(int t)
 {
 	std::string filename("testfiles/test");
 	filename += uploads[t].fnextra;
-	printf("%s\n", filename.c_str());
+	BOX_TRACE("Writing " << filename);
 	
 	FileStream write(filename.c_str(), O_WRONLY | O_CREAT);
 	
@@ -457,9 +456,9 @@ void test_test_file(int t, IOStream &rStream)
 	TEST_THAT(unlink("testfiles/test_download") == 0);
 }
 
-void test_everything_deleted(BackupProtocolCallable &protocol, int64_t DirID)
+void assert_everything_deleted(BackupProtocolCallable &protocol, int64_t DirID)
 {
-	printf("Test for del: %llx\n", (unsigned long long)DirID);
+	BOX_TRACE("Test for del: " << BOX_FORMAT_OBJECTID(DirID));
 	
 	// Command
 	std::auto_ptr<BackupProtocolSuccess> dirreply(protocol.QueryListDirectory(
@@ -478,7 +477,7 @@ void test_everything_deleted(BackupProtocolCallable &protocol, int64_t DirID)
 		{
 			dirs++;
 			// Recurse
-			test_everything_deleted(protocol, en->GetObjectID());
+			assert_everything_deleted(protocol, en->GetObjectID());
 		}
 		else
 		{
@@ -486,7 +485,7 @@ void test_everything_deleted(BackupProtocolCallable &protocol, int64_t DirID)
 		}
 
 		// Check it's deleted
-		TEST_THAT(en->GetFlags() & BackupProtocolListDirectory::Flags_Deleted);
+		TEST_THAT(en->IsDeleted());
 	}
 	
 	// Check there were the right number of files and directories
@@ -571,10 +570,10 @@ void check_dir_after_uploads(BackupProtocolCallable &protocol,
 {
 	// Command
 	std::auto_ptr<BackupProtocolSuccess> dirreply(protocol.QueryListDirectory(
-			BackupProtocolListDirectory::RootDirectory,
+			BACKUPSTORE_ROOT_DIRECTORY_ID,
 			BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 			BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, false /* no attributes */));
-	TEST_THAT(dirreply->GetObjectID() == BackupProtocolListDirectory::RootDirectory);
+	TEST_THAT(dirreply->GetObjectID() == BACKUPSTORE_ROOT_DIRECTORY_ID);
 	// Stream
 	BackupStoreDirectory dir(protocol.ReceiveStream(), SHORT_TIMEOUT);
 	TEST_EQUAL(UPLOAD_NUM, dir.GetNumberOfEntries());
@@ -799,7 +798,9 @@ bool test_server_housekeeping()
 	BackupStoreFilenameClear store1name("file1");
 	{
 		FileStream out("testfiles/file1_upload1", O_WRONLY | O_CREAT);
-		std::auto_ptr<IOStream> encoded(BackupStoreFile::EncodeFile("testfiles/file1", BackupProtocolListDirectory::RootDirectory, store1name));
+		std::auto_ptr<IOStream> encoded(
+			BackupStoreFile::EncodeFile("testfiles/file1",
+				BACKUPSTORE_ROOT_DIRECTORY_ID, store1name));
 		encoded->CopyStreamTo(out);
 	}
 
@@ -816,7 +817,7 @@ bool test_server_housekeeping()
 	{
 		std::auto_ptr<IOStream> upload(new FileStream("testfiles/file1_upload1"));
 		std::auto_ptr<BackupProtocolSuccess> stored(protocol.QueryStoreFile(
-			BackupProtocolListDirectory::RootDirectory,
+			BACKUPSTORE_ROOT_DIRECTORY_ID,
 			0x123456789abcdefLL,		/* modification time */
 			0x7362383249872dfLL,		/* attr hash */
 			0,							/* diff from ID */
@@ -833,8 +834,8 @@ bool test_server_housekeeping()
 	// And retrieve it
 	{
 		// Retrieve as object
-		std::auto_ptr<BackupProtocolSuccess> getfile(protocol.QueryGetObject(store1objid));
-		TEST_THAT(getfile->GetObjectID() == store1objid);
+		COMMAND(QueryGetObject(store1objid), store1objid);
+
 		// BLOCK
 		{
 			// Get stream
@@ -849,8 +850,8 @@ bool test_server_housekeeping()
 		}
 
 		// Retrieve as file
-		std::auto_ptr<BackupProtocolSuccess> getobj(protocol.QueryGetFile(BackupProtocolListDirectory::RootDirectory, store1objid));
-		TEST_THAT(getobj->GetObjectID() == store1objid);
+		COMMAND(QueryGetFile(BACKUPSTORE_ROOT_DIRECTORY_ID, store1objid), store1objid);
+
 		// BLOCK
 		{
 			UNLINK_IF_EXISTS("testfiles/file1_upload_retrieved_str");
@@ -887,7 +888,7 @@ bool test_server_housekeeping()
 
 		// and again, by name
 		{
-			std::auto_ptr<BackupProtocolSuccess> getblockindex(protocol.QueryGetBlockIndexByName(BackupProtocolListDirectory::RootDirectory, store1name));
+			std::auto_ptr<BackupProtocolSuccess> getblockindex(protocol.QueryGetBlockIndexByName(BACKUPSTORE_ROOT_DIRECTORY_ID, store1name));
 			TEST_THAT(getblockindex->GetObjectID() == store1objid);
 			std::auto_ptr<IOStream> blockIndexStream(protocol.ReceiveStream());
 			// Check against uploaded file
@@ -899,7 +900,7 @@ bool test_server_housekeeping()
 	{
 		// Command
 		std::auto_ptr<BackupProtocolSuccess> dirreply(protocol.QueryListDirectory(
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 				BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, false /* no attributes */));
 		// Stream
@@ -928,7 +929,7 @@ bool test_server_housekeeping()
 	// Upload again, as a patch to the original file.
 	int64_t patch1_id = BackupStoreFile::QueryStoreFileDiff(protocol,
 		"testfiles/file1", // LocalFilename
-		BackupProtocolListDirectory::RootDirectory, // DirectoryObjectID
+		BACKUPSTORE_ROOT_DIRECTORY_ID, // DirectoryObjectID
 		store1objid, // DiffFromFileID
 		0x7362383249872dfLL, // AttributesHash
 		store1name // StoreFilename
@@ -958,7 +959,7 @@ bool test_server_housekeeping()
 
 	int64_t patch2_id = BackupStoreFile::QueryStoreFileDiff(protocol,
 		"testfiles/file1", // LocalFilename
-		BackupProtocolListDirectory::RootDirectory, // DirectoryObjectID
+		BACKUPSTORE_ROOT_DIRECTORY_ID, // DirectoryObjectID
 		patch1_id, // DiffFromFileID
 		0x7362383249872dfLL, // AttributesHash
 		store1name // StoreFilename
@@ -991,7 +992,7 @@ bool test_server_housekeeping()
 	// but used to not adjust the number of old/deleted files properly.
 	int64_t replaced_id = BackupStoreFile::QueryStoreFileDiff(protocol,
 		"testfiles/file1", // LocalFilename
-		BackupProtocolListDirectory::RootDirectory, // DirectoryObjectID
+		BACKUPSTORE_ROOT_DIRECTORY_ID, // DirectoryObjectID
 		0, // DiffFromFileID
 		0x7362383249872dfLL, // AttributesHash
 		store1name // StoreFilename
@@ -1041,7 +1042,7 @@ bool test_server_housekeeping()
 
 	// Check that deleting files is accounted for as well
 	protocol.QueryDeleteFile(
-		BackupProtocolListDirectory::RootDirectory, // InDirectory
+		BACKUPSTORE_ROOT_DIRECTORY_ID, // InDirectory
 		store1name); // Filename
 
 	// The old version file is deleted as well!
@@ -1080,8 +1081,10 @@ bool test_server_housekeeping()
 
 	// Try using GetFile on a directory
 	{
-		TEST_CHECK_THROWS(std::auto_ptr<BackupProtocolSuccess> getFile(protocol.QueryGetFile(BackupProtocolListDirectory::RootDirectory, BackupProtocolListDirectory::RootDirectory)),
-			ConnectionException, Protocol_UnexpectedReply);
+		int64_t subdirid = create_directory(protocol);
+		TEST_COMMAND_RETURNS_ERROR(protocol,
+			QueryGetFile(BACKUPSTORE_ROOT_DIRECTORY_ID, subdirid),
+			Err_FileDoesNotVerify);
 	}
 
 	// Try retrieving an object that doesn't exist. That used to return
@@ -1093,7 +1096,7 @@ bool test_server_housekeeping()
 	protocol.QueryFinished();
 	TEST_THAT(run_housekeeping_and_check_account());
 
-	ExpectedRefCounts.resize(2); // stop test failure in tearDown()
+	ExpectedRefCounts.resize(3); // stop test failure in tearDown()
 	tearDown();
 	return true;
 }
@@ -1108,7 +1111,7 @@ int64_t create_directory(BackupProtocolCallable& protocol, int64_t parent_dir_id
 	std::auto_ptr<BackupProtocolSuccess> dirCreate(protocol.QueryCreateDirectory(
 		parent_dir_id, FAKE_ATTR_MODIFICATION_TIME, dirname, attr));
 
-	int64_t subdirid = dirCreate->GetObjectID(); 
+	int64_t subdirid = dirCreate->GetObjectID();
 	set_refcount(subdirid, 1);
 	return subdirid;
 }
@@ -1152,8 +1155,8 @@ bool assert_writable_connection_fails(BackupProtocolCallable& protocol)
 	std::auto_ptr<BackupProtocolVersion> serverVersion
 		(protocol.QueryVersion(BACKUP_STORE_SERVER_VERSION));
 	TEST_THAT(serverVersion->GetVersion() == BACKUP_STORE_SERVER_VERSION);
-	TEST_CHECK_THROWS(protocol.QueryLogin(0x01234567, 0),
-		ConnectionException, Protocol_UnexpectedReply);
+	TEST_COMMAND_RETURNS_ERROR_OR(protocol, QueryLogin(0x01234567, 0),
+		Err_CannotLockStoreForWriting, return false);
 	protocol.QueryFinished();
 	return true;
 }
@@ -1195,7 +1198,7 @@ bool test_multiple_uploads()
 		// Command
 		std::auto_ptr<BackupProtocolSuccess> dirreply(
 			apProtocol->QueryListDirectory(
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 				BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, false /* no attributes */));
 		// Stream
@@ -1208,7 +1211,7 @@ bool test_multiple_uploads()
 	// Command
 	std::auto_ptr<BackupProtocolSuccess> dirreply(
 		protocolReadOnly.QueryListDirectory(
-			BackupProtocolListDirectory::RootDirectory,
+			BACKUPSTORE_ROOT_DIRECTORY_ID,
 			BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 			BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING,
 			false /* no attributes */));
@@ -1217,7 +1220,7 @@ bool test_multiple_uploads()
 		protocolReadOnly.GetTimeout());
 	TEST_THAT(dir.GetNumberOfEntries() == 0);			
 
-	// BLOCK
+	// TODO FIXME dedent
 	{
 		TEST_THAT(check_num_files(0, 0, 0, 1));
 
@@ -1234,11 +1237,11 @@ bool test_multiple_uploads()
 			filename += uploads[t].fnextra;
 			int64_t modtime = 0;
 
-			std::auto_ptr<IOStream> upload(BackupStoreFile::EncodeFile(filename.c_str(), BackupProtocolListDirectory::RootDirectory, uploads[t].name, &modtime));
+			std::auto_ptr<IOStream> upload(BackupStoreFile::EncodeFile(filename.c_str(), BACKUPSTORE_ROOT_DIRECTORY_ID, uploads[t].name, &modtime));
 			TEST_THAT(modtime != 0);
 			
 			std::auto_ptr<BackupProtocolSuccess> stored(apProtocol->QueryStoreFile(
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				modtime,
 				modtime, /* use it for attr hash too */
 				0, /* diff from ID */
@@ -1277,7 +1280,7 @@ bool test_multiple_uploads()
 			std::auto_ptr<IOStream> attrnew(
 				new MemBlockStream(attr3, sizeof(attr3)));
 			std::auto_ptr<BackupProtocolSuccess> set(apProtocol->QuerySetReplacementFileAttributes(
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				32498749832475LL,
 				uploads[UPLOAD_ATTRS_EN].name,
 				attrnew));
@@ -1288,7 +1291,7 @@ bool test_multiple_uploads()
 		// Delete one of them (will implicitly delete an old version)
 		{
 			std::auto_ptr<BackupProtocolSuccess> del(apProtocol->QueryDeleteFile(
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				uploads[UPLOAD_DELETE_EN].name));
 			TEST_THAT(del->GetObjectID() == uploads[UPLOAD_DELETE_EN].allocated_objid);
 			TEST_THAT(check_num_files(UPLOAD_NUM - 4, 3, 2, 1));
@@ -1305,7 +1308,7 @@ bool test_multiple_uploads()
 			}
 			// query index and test
 			std::auto_ptr<BackupProtocolSuccess> getblockindex(apProtocol->QueryGetBlockIndexByName(
-				BackupProtocolListDirectory::RootDirectory, uploads[UPLOAD_DELETE_EN].name));
+				BACKUPSTORE_ROOT_DIRECTORY_ID, uploads[UPLOAD_DELETE_EN].name));
 			TEST_THAT(getblockindex->GetObjectID() == uploads[UPLOAD_DELETE_EN].allocated_objid);
 			std::auto_ptr<IOStream> blockIndexStream(apProtocol->ReceiveStream());
 			TEST_THAT(check_block_index("testfiles/downloaddelobj", *blockIndexStream));
@@ -1315,7 +1318,7 @@ bool test_multiple_uploads()
 		for(int t = 0; t < UPLOAD_NUM; ++t)
 		{
 			printf("%d\n", t);
-			std::auto_ptr<BackupProtocolSuccess> getFile(apProtocol->QueryGetFile(BackupProtocolListDirectory::RootDirectory, uploads[t].allocated_objid));
+			std::auto_ptr<BackupProtocolSuccess> getFile(apProtocol->QueryGetFile(BACKUPSTORE_ROOT_DIRECTORY_ID, uploads[t].allocated_objid));
 			TEST_THAT(getFile->GetObjectID() == uploads[t].allocated_objid);
 			std::auto_ptr<IOStream> filestream(apProtocol->ReceiveStream());
 			test_test_file(t, *filestream);
@@ -1377,7 +1380,7 @@ bool test_multiple_uploads()
 		{
 			// Fetch the block index for this one
 			std::auto_ptr<BackupProtocolSuccess> getblockindex(apProtocol->QueryGetBlockIndexByName(
-				BackupProtocolListDirectory::RootDirectory, uploads[UPLOAD_PATCH_EN].name));
+				BACKUPSTORE_ROOT_DIRECTORY_ID, uploads[UPLOAD_PATCH_EN].name));
 			TEST_THAT(getblockindex->GetObjectID() == uploads[UPLOAD_PATCH_EN].allocated_objid);
 			std::auto_ptr<IOStream> blockIndexStream(apProtocol->ReceiveStream());
 			
@@ -1387,7 +1390,7 @@ bool test_multiple_uploads()
 			std::auto_ptr<IOStream> patchstream(
 				BackupStoreFile::EncodeFileDiff(
 					TEST_FILE_FOR_PATCHING ".mod", 
-					BackupProtocolListDirectory::RootDirectory,
+					BACKUPSTORE_ROOT_DIRECTORY_ID,
 					uploads[UPLOAD_PATCH_EN].name, 
 					uploads[UPLOAD_PATCH_EN].allocated_objid, 
 					*blockIndexStream,
@@ -1407,7 +1410,7 @@ bool test_multiple_uploads()
 			{
 				std::auto_ptr<IOStream> uploadpatch(new FileStream(TEST_FILE_FOR_PATCHING ".patch"));
 				std::auto_ptr<BackupProtocolSuccess> stored(apProtocol->QueryStoreFile(
-					BackupProtocolListDirectory::RootDirectory,
+					BACKUPSTORE_ROOT_DIRECTORY_ID,
 					modtime,
 					modtime, /* use it for attr hash too */
 					uploads[UPLOAD_PATCH_EN].allocated_objid,		/* diff from ID */
@@ -1421,7 +1424,7 @@ bool test_multiple_uploads()
 			set_refcount(patchedID, 1);
 
 			// Then download it to check it's OK
-			std::auto_ptr<BackupProtocolSuccess> getFile(apProtocol->QueryGetFile(BackupProtocolListDirectory::RootDirectory, patchedID));
+			std::auto_ptr<BackupProtocolSuccess> getFile(apProtocol->QueryGetFile(BACKUPSTORE_ROOT_DIRECTORY_ID, patchedID));
 			TEST_THAT(getFile->GetObjectID() == patchedID);
 			std::auto_ptr<IOStream> filestream(apProtocol->ReceiveStream());
 			BackupStoreFile::DecodeFile(*filestream,
@@ -1439,12 +1442,13 @@ bool test_multiple_uploads()
 		int64_t subdirfileid = create_file(*apProtocol, subdirid);
 		TEST_THAT(check_num_files(UPLOAD_NUM - 3, 4, 2, 2));
 
-		printf("\n==== Checking upload using read-only connection\n");
+		BOX_TRACE("Checking upload using read-only connection");
 		// Check the directories on the read only connection
 		{
 			// Command
-			std::auto_ptr<BackupProtocolSuccess> dirreply(protocolReadOnly.QueryListDirectory(
-					BackupProtocolListDirectory::RootDirectory,
+			std::auto_ptr<BackupProtocolSuccess> dirreply(
+				protocolReadOnly.QueryListDirectory(
+					BACKUPSTORE_ROOT_DIRECTORY_ID,
 					BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 					BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, false /* no attributes! */)); // Stream
 			BackupStoreDirectory dir(protocolReadOnly.ReceiveStream(),
@@ -1468,16 +1472,19 @@ bool test_multiple_uploads()
 				}
 				en = t;
 			}
-			// Does it look right?
+
+			// Check that the last entry looks right
 			TEST_THAT(en->GetName() == dirname);
 			TEST_THAT(en->GetFlags() == BackupProtocolListDirectory::Flags_Dir);
 			TEST_THAT(en->GetObjectID() == subdirid);
 			TEST_THAT(en->GetModificationTime() == 0);	// dirs don't have modification times.
 		}
 
+		BOX_TRACE("Checking subdirectory using read-only connection");
 		{
 			// Command
-			std::auto_ptr<BackupProtocolSuccess> dirreply(protocolReadOnly.QueryListDirectory(
+			std::auto_ptr<BackupProtocolSuccess> dirreply(
+				protocolReadOnly.QueryListDirectory(
 					subdirid,
 					BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 					BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, true /* get attributes */));
@@ -1506,9 +1513,8 @@ bool test_multiple_uploads()
 			StreamableMemBlock attr(attr1, sizeof(attr1));
 			TEST_THAT(dir.GetAttributes() == attr);
 		}
-		printf("done.\n\n");
 
-		// Check that we don't get attributes if we don't ask for them
+		BOX_TRACE("Checking that we don't get attributes if we don't ask for them");
 		{
 			// Command
 			std::auto_ptr<BackupProtocolSuccess> dirreply(protocolReadOnly.QueryListDirectory(
@@ -1556,7 +1562,11 @@ bool test_multiple_uploads()
 			TEST_THAT(dir.GetAttributes() == attrtest);
 		}
 		
-		// sleep to ensure that the timestamp on the file will change
+		// Sleep before modifying the root directory, to ensure that
+		// the timestamp on the file it's stored in will change when
+		// we modify it, invalidating the read-only connection's cache
+		// and forcing it to reload the root directory, next time we
+		// ask for its contents.
 		::safe_sleep(1);
 
 		// Test moving a file
@@ -1564,7 +1574,7 @@ bool test_multiple_uploads()
 			BackupStoreFilenameClear newName("moved-files");
 		
 			std::auto_ptr<BackupProtocolSuccess> rep(apProtocol->QueryMoveObject(uploads[UPLOAD_FILE_TO_MOVE].allocated_objid,
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				subdirid, BackupProtocolMoveObject::Flags_MoveAllWithSameName, newName));
 			TEST_THAT(rep->GetObjectID() == uploads[UPLOAD_FILE_TO_MOVE].allocated_objid);
 		}
@@ -1619,10 +1629,11 @@ bool test_multiple_uploads()
 					subdirid,
 					BackupProtocolListDirectory::Flags_INCLUDE_EVERYTHING,
 					BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, false /* no attributes */));
+
 			// Stream
-			BackupStoreDirectory dir;
-			std::auto_ptr<IOStream> dirstream(protocolReadOnly.ReceiveStream());
-			dir.ReadFromStream(*dirstream, SHORT_TIMEOUT);
+			BackupStoreDirectory dir(protocolReadOnly.ReceiveStream(),
+				SHORT_TIMEOUT);
+
 			// Check entries
 			BackupStoreDirectory::Iterator i(dir);
 			BackupStoreDirectory::Entry *en = 0;
@@ -1658,7 +1669,7 @@ bool test_multiple_uploads()
 			BackupStoreFilenameClear file2("file2");
 			std::auto_ptr<IOStream> upload(
 				BackupStoreFile::EncodeFile("testfiles/test2",
-					BackupProtocolListDirectory::RootDirectory, file2));
+					BACKUPSTORE_ROOT_DIRECTORY_ID, file2));
 			std::auto_ptr<BackupProtocolSuccess> stored(apProtocol->QueryStoreFile(
 				subsubdirid,
 				0x123456789abcdefLL,		/* modification time */
@@ -1738,7 +1749,7 @@ bool test_multiple_uploads()
 		// Create some nice recursive directories
 		TEST_THAT(check_reference_counts());
 		int64_t dirtodelete = create_test_data_subdirs(*apProtocol,
-			BackupProtocolListDirectory::RootDirectory,
+			BACKUPSTORE_ROOT_DIRECTORY_ID,
 			"test_delete", 6 /* depth */, apRefCount.get());
 		TEST_THAT(check_reference_counts());
 		
@@ -1764,7 +1775,7 @@ bool test_multiple_uploads()
 		{
 			// Command
 			std::auto_ptr<BackupProtocolSuccess> dirreply(protocolReadOnly.QueryListDirectory(
-					BackupProtocolListDirectory::RootDirectory,
+					BACKUPSTORE_ROOT_DIRECTORY_ID,
 					BackupProtocolListDirectory::Flags_Dir | BackupProtocolListDirectory::Flags_Deleted,
 					BackupProtocolListDirectory::Flags_EXCLUDE_NOTHING, false /* no attributes */));
 			// Stream
@@ -1785,7 +1796,7 @@ bool test_multiple_uploads()
 			}
 			
 			// Then... check everything's deleted
-			test_everything_deleted(protocolReadOnly, dirtodelete);
+			assert_everything_deleted(protocolReadOnly, dirtodelete);
 		}
 
 		// Finish the connections
@@ -1847,7 +1858,9 @@ bool test_directory_parent_entry_tracks_directory_size()
 	TEST_EQUAL(old_size, get_object_size(protocolReadOnly, subdirid,
 		BACKUPSTORE_ROOT_DIRECTORY_ID));
 
-	// TODO FIXME Sleep to ensure that file timestamp changes (win32?)
+	// Sleep to ensure that the directory file timestamp changes, so that
+	// the read-only connection will discard its cached copy.
+	safe_sleep(1);
 
 	// Start adding files until the size on disk increases. This is
 	// guaranteed to happen eventually :)
@@ -2363,17 +2376,9 @@ bool test_login_with_disabled_account()
 		TEST_THAT(serverVersion->GetVersion() == BACKUP_STORE_SERVER_VERSION);
 
 		// Login
-		TEST_CHECK_THROWS(std::auto_ptr<BackupProtocolLoginConfirmed>
-			loginConf(protocol.QueryLogin(0x01234567, 0)),
-			ConnectionException, Protocol_UnexpectedReply);
-		int type, subType;
-		TEST_EQUAL_LINE(true, protocol.GetLastError(type, subType),
-			"expected a protocol error");
-		TEST_EQUAL_LINE(BackupProtocolError::ErrorType, type,
-			"expected a BackupProtocolError");
-		TEST_EQUAL_LINE(BackupProtocolError::Err_DisabledAccount, subType,
-			"expected an Err_DisabledAccount");
-		
+		TEST_COMMAND_RETURNS_ERROR(protocol, QueryLogin(0x01234567, 0),
+			BackupProtocolError::Err_DisabledAccount);
+
 		// Finish the connection
 		protocol.QueryFinished();
 	}
@@ -2403,6 +2408,7 @@ bool test_login_with_no_refcount_db()
 		"test", "backup/01234567/", 0, false), // Not read-only
 		BackupStoreException, CorruptReferenceCountDatabase);
 
+	// Run housekeeping, check that it fixes the refcount db
 	std::auto_ptr<BackupStoreAccountDatabase> apAccounts(
 		BackupStoreAccountDatabase::Read("testfiles/accounts.txt"));
 	BackupStoreAccountDatabase::Entry account =
@@ -2458,29 +2464,29 @@ bool test_housekeeping_deletes_files()
 	// Create some nice recursive directories
 	write_test_file(1);
 	int64_t dirtodelete = create_test_data_subdirs(protocolLocal,
-		BackupProtocolListDirectory::RootDirectory, "test_delete", 6 /* depth */,
+		BACKUPSTORE_ROOT_DIRECTORY_ID, "test_delete", 6 /* depth */,
 		NULL /* pRefCount */);
 
 	TEST_EQUAL(dirtodelete,
 		protocolLocal.QueryDeleteDirectory(dirtodelete)->GetObjectID());
-	test_everything_deleted(protocolLocal, dirtodelete);
+	assert_everything_deleted(protocolLocal, dirtodelete);
 	protocolLocal.QueryFinished();
 
 	// First, things as they are now.
 	TEST_THAT_ABORTONFAIL(StartServer());
 	recursive_count_objects_results before = {0,0,0};
-	recursive_count_objects("localhost", BackupProtocolListDirectory::RootDirectory, before);
+	recursive_count_objects("localhost", BACKUPSTORE_ROOT_DIRECTORY_ID, before);
 	
-	TEST_THAT(before.objectsNotDel == 0);
+	TEST_EQUAL(0, before.objectsNotDel);
 	TEST_THAT(before.deleted != 0);
 	TEST_THAT(before.old != 0);
 
 	// Kill it
 	TEST_THAT(StopServer());
 
-	// Set a new limit on the account -- leave the hard limit 
-	// high to make sure the target for freeing space is the 
-	// soft limit.
+	// Reduce the store limits, so housekeeping will remove all old files.
+	// Leave the hard limit high, so we know that housekeeping's target
+	// for freeing space is the soft limit.
 	TEST_THAT_ABORTONFAIL(::system(BBSTOREACCOUNTS 
 		" -c testfiles/bbstored.conf setlimit 01234567 "
 		"10B 20000B") == 0);
@@ -2560,7 +2566,7 @@ bool test_account_limits_respected()
 		std::auto_ptr<IOStream> attr(new MemBlockStream(&modtime, sizeof(modtime)));
 		BackupStoreFilenameClear fnxd("exceed-limit-dir");
 		TEST_CHECK_THROWS(protocol.QueryCreateDirectory(
-				BackupProtocolListDirectory::RootDirectory,
+				BACKUPSTORE_ROOT_DIRECTORY_ID,
 				FAKE_ATTR_MODIFICATION_TIME, fnxd, attr),
 			ConnectionException, Protocol_UnexpectedReply);
 
diff --git a/test/basicserver/TestCommands.cpp b/test/basicserver/TestCommands.cpp
index 7c22eab3..63425f48 100644
--- a/test/basicserver/TestCommands.cpp
+++ b/test/basicserver/TestCommands.cpp
@@ -72,8 +72,9 @@ std::auto_ptr<TestProtocolMessage> TestProtocolGetStream::DoCommand(TestProtocol
 	return std::auto_ptr<TestProtocolMessage>(new TestProtocolGetStream(mStartingValue, mUncertainSize));
 }
 
-std::auto_ptr<TestProtocolMessage> TestProtocolSendStream::DoCommand(TestProtocolReplyable &rProtocol,
-	TestContext &rContext, IOStream& rDataStream) const
+std::auto_ptr<TestProtocolMessage> TestProtocolSendStream::DoCommand(
+	TestProtocolReplyable &rProtocol, TestContext &rContext,
+	IOStream& rDataStream) const
 {
 	if(mValue != 0x73654353298ffLL)
 	{
diff --git a/test/bbackupd/testbbackupd.cpp b/test/bbackupd/testbbackupd.cpp
index c18a9ead..304e5fec 100644
--- a/test/bbackupd/testbbackupd.cpp
+++ b/test/bbackupd/testbbackupd.cpp
@@ -690,7 +690,7 @@ void do_interrupted_restore(const TLSContext &context, int64_t restoredirid)
 #endif // !WIN32
 
 #ifdef WIN32
-bool set_file_time(const char* filename, FILETIME creationTime, 
+bool set_file_time(const char* filename, FILETIME creationTime,
 	FILETIME lastModTime, FILETIME lastAccessTime)
 {
 	HANDLE handle = openfile(filename, O_RDWR, 0);
@@ -706,7 +706,7 @@ bool set_file_time(const char* filename, FILETIME creationTime,
 }
 #endif
 
-void intercept_setup_delay(const char *filename, unsigned int delay_after, 
+void intercept_setup_delay(const char *filename, unsigned int delay_after,
 	int delay_ms, int syscall_to_delay);
 bool intercept_triggered();
 
@@ -714,7 +714,7 @@ int64_t SearchDir(BackupStoreDirectory& rDir,
 	const std::string& rChildName)
 {
 	BackupStoreDirectory::Iterator i(rDir);
-	BackupStoreFilenameClear child(rChildName.c_str());
+	BackupStoreFilenameClear child(rChildName);
 	BackupStoreDirectory::Entry *en = i.FindMatchingClearName(child);
 	if (en == 0) return 0;
 	int64_t id = en->GetObjectID();
@@ -1184,8 +1184,7 @@ int test_bbackupd()
 			std::string line;
 			TEST_THAT(reader.GetLine(line));
 			std::string comp = "Receive Success(0x";
-			TEST_EQUAL_LINE(comp, line.substr(0, comp.size()),
-				line);
+			TEST_EQUAL_LINE(comp, line.substr(0, comp.size()), line);
 			TEST_THAT(reader.GetLine(line));
 			TEST_EQUAL("Receiving stream, size 124", line);
 			TEST_THAT(reader.GetLine(line));
@@ -1459,7 +1458,10 @@ int test_bbackupd()
 	// 0000000c f----- 00002 Test1/spacetest/d3/d4/f5
 	// 0000000d -d---- 00002 Test1/spacetest/d6
 	// 0000000e -d---- 00002 Test1/spacetest/d7
-	// This is 28 blocks total.
+	// 0000000f f--o-- 00002 Test1/spacetest/f1
+	// 00000010 f--o-- 00002 Test1/spacetest/f1
+	// 00000011 f----- 00002 Test1/spacetest/f1
+	// This is 34 blocks total.
 
 	// BLOCK
 	{
@@ -1520,7 +1522,7 @@ int test_bbackupd()
 		TEST_THAT(::unlink("testfiles/TestDir1/spacetest/f1") == 0);
 		TEST_THAT(::system("rm -rf testfiles/TestDir1/spacetest/d7") == 0);
 
-		// The following files should be on the server:
+		// The following files should be in the backup directory:
 		// 00000001 -d---- 00002 (root)
 		// 00000002 -d---- 00002 Test1
 		// 00000003 -d---- 00002 Test1/spacetest
@@ -1530,22 +1532,26 @@ int test_bbackupd()
 		// 00000007 f----- 00002 Test1/spacetest/d1/f3
 		// 00000008 f----- 00002 Test1/spacetest/d1/f4
 		// 00000009 -d---- 00002 Test1/spacetest/d2
+		// 00000009 -d---- 00002 Test1/spacetest/d6
 		// 0000000a -d---- 00002 Test1/spacetest/d3
 		// 0000000b -d---- 00002 Test1/spacetest/d3/d4
 		// 0000000c f----- 00002 Test1/spacetest/d3/d4/f5
 		// 0000000d -d---- 00002 Test1/spacetest/d6
+		// 0000000d -d---- 00002 Test1/spacetest/d6/d8
+		// 0000000d -d---- 00002 Test1/spacetest/d6/d8/f7
 		// 0000000e -dX--- 00002 Test1/spacetest/d7
-		// This is 28 blocks total, of which 2 in deleted files
+		//
+		// root + location + spacetest1 + spacetest2 = 17 files
+		// = 34 blocks with raidfile. Of which 2 in deleted files
 		// and 18 in directories. Note that f1 and d7 may or may
 		// not be deleted yet.
 		//
-		// spacetest1 + spacetest2 = 16 files = 32 blocks with raidfile
-		// minus one file and one dir is 28 blocks
-		//
-		// d2/f6, d6/d8 and d6/d8/f7 are new
-		// even if the client marks f1 and d7 as deleted, and
-		// housekeeping deleted them, the backup cannot complete
-		// if the limit is 20 blocks.
+		// The files and dirs from spacetest1 are already on the server
+		// (28 blocks). If we set the limit to 20 then the client will
+		// notice as soon as it tries to create the new files and dirs
+		// from spacetest2. It should still delete f1 and d7, but that
+		// won't bring it back under the hard limit, so no files from
+		// spacetest2 should be uploaded.
 
 		BOX_TRACE("Waiting for sync for bbackupd to notice that the "
 			"store is full");
@@ -1554,7 +1560,6 @@ int test_bbackupd()
 
 		BOX_TRACE("Compare to check that there are differences");
 		TEST_COMPARE(Compare_Different);
-		BOX_TRACE("Compare finished.");
 
 		// Check that the notify script was run
 		TEST_THAT(TestFileExists("testfiles/notifyran.store-full.1"));
@@ -1619,16 +1624,13 @@ int test_bbackupd()
 		// But only twice!
 		// TEST_THAT(!TestFileExists("testfiles/notifyran.store-full.3"));
 
-		// All these should be marked as deleted but hopefully
-		// not removed by housekeeping yet:
+		// All these should be marked as deleted but not removed by
+		// housekeeping yet:
 		// f1		deleted
 		// f2		excluded
-		// d1		excluded (why?)
-		// d1/f3	excluded (why?)
 		// d3		excluded
 		// d3/d4	excluded
 		// d3/d4/f5	excluded
-		// d7		deleted
 		// Careful with timing here, these files will be removed by
 		// housekeeping the next time it runs. On Win32, housekeeping
 		// runs immediately after disconnect, but only if enough time
@@ -1697,7 +1699,6 @@ int test_bbackupd()
 			// Log out.
 			client->QueryFinished();
 		}
-		BOX_TRACE("done.");
 
 		if (failures) return 1;
 
@@ -1711,7 +1712,7 @@ int test_bbackupd()
 			"deleted files");
 #endif
 
-		BOX_TRACE("Check that the files were removed");
+		BOX_INFO("Checking that the files were removed");
 		{
 			std::auto_ptr<BackupProtocolCallable> client =
 				connect_and_login(context, 0 /* read-write */);
@@ -1737,7 +1738,6 @@ int test_bbackupd()
 			TEST_THAT(SearchDir(*spacetest_dir, "d3") == 0);
 			TEST_THAT(SearchDir(*spacetest_dir, "d7") == 0);
 
-
 			// f2, d3, d3/d4 and d3/d4/f5 have been removed.
 			// The files were counted as deleted files before, the
 			// deleted directories just as directories.
@@ -1987,10 +1987,12 @@ int test_bbackupd()
 		char buf[PATH_MAX];
 		TEST_THAT(getcwd(buf, sizeof(buf)) == buf);
 		std::string path = buf;
-		path += DIRECTORY_SEPARATOR SYM_DIR 
+		// testfiles/TestDir1/symlink_test/a/subdir ->
+		// testfiles/TestDir1/symlink_test/b/link
+		path += DIRECTORY_SEPARATOR SYM_DIR
 			DIRECTORY_SEPARATOR "a"
 			DIRECTORY_SEPARATOR "subdir";
-		TEST_THAT(symlink(path.c_str(), SYM_DIR 
+		TEST_THAT(symlink(path.c_str(), SYM_DIR
 			DIRECTORY_SEPARATOR "b"
 			DIRECTORY_SEPARATOR "link") == 0);
 
@@ -2267,6 +2269,7 @@ int test_bbackupd()
 
 		{
 			#ifdef WIN32
+				// Cygwin chmod changes Windows file attributes
 				TEST_THAT(::system("chmod 0555 testfiles/"
 					"TestDir1/x1") == 0);
 			#else
@@ -2607,7 +2610,6 @@ int test_bbackupd()
 			TEST_THAT(unlink(sync_control_file) == 0);
 			wait_for_sync_start();
 			long end_time = time(NULL);
-
 			long wait_time = end_time - start_time + 2;
 
 			// should be about 10 seconds
@@ -2645,15 +2647,15 @@ int test_bbackupd()
 
 		#ifndef WIN32
 			// New symlink
-			TEST_THAT(::symlink("does-not-exist", 
+			TEST_THAT(::symlink("does-not-exist",
 				"testfiles/TestDir1/symlink-to-dir") == 0);
 		#endif		
 
 		// Update a file (will be uploaded as a diff)
 		{
-			// Check that the file is over the diffing 
+			// Check that the file is over the diffing
 			// threshold in the bbackupd.conf file
-			TEST_THAT(TestGetFileSize("testfiles/TestDir1/f45.df") 
+			TEST_THAT(TestGetFileSize("testfiles/TestDir1/f45.df")
 				> 1024);
 			
 			// Add a bit to the end
@@ -2661,7 +2663,7 @@ int test_bbackupd()
 			TEST_THAT(f != 0);
 			::fprintf(f, "EXTRA STUFF");
 			::fclose(f);
-			TEST_THAT(TestGetFileSize("testfiles/TestDir1/f45.df") 
+			TEST_THAT(TestGetFileSize("testfiles/TestDir1/f45.df")
 				> 1024);
 		}
 
@@ -2685,8 +2687,7 @@ int test_bbackupd()
 
 		// Check that store errors are reported neatly
 		printf("\n==== Create store error\n");
-		TEST_THAT(system("rm -f testfiles/notifyran.backup-error.*")
-			== 0);
+		TEST_THAT(system("rm -f testfiles/notifyran.backup-error.*") == 0);
 
 		// Break the store. We need a write lock on the account
 		// while we do this, otherwise housekeeping might be running
@@ -2896,12 +2897,12 @@ int test_bbackupd()
 				== 0);
 		#endif
 
-		TEST_THAT(::mkdir("testfiles/TestDir1/symlink-to-dir", 0755) 
+		TEST_THAT(::mkdir("testfiles/TestDir1/symlink-to-dir", 0755)
 			== 0);
-		TEST_THAT(::mkdir("testfiles/TestDir1/x1/dir-to-file", 0755) 
+		TEST_THAT(::mkdir("testfiles/TestDir1/x1/dir-to-file", 0755)
 			== 0);
 
-		// NOTE: create a file within the directory to 
+		// NOTE: create a file within the directory to
 		// avoid deletion by the housekeeping process later
 
 		#ifndef WIN32
@@ -2945,8 +2946,7 @@ int test_bbackupd()
 		if (failures) return 1;
 
 		// And then, put it back to how it was before.
-		printf("\n==== Replace symlink with directory "
-			"(which was a symlink)\n");
+		BOX_INFO("Replace symlink with directory (which was a symlink)");
 
 		#ifndef WIN32
 			TEST_THAT(::unlink("testfiles/TestDir1/x1"
@@ -3047,8 +3047,8 @@ int test_bbackupd()
 
 		// case which went wrong: rename a tracked file over an
 		// existing tracked file
-		printf("\n==== Rename over existing tracked file\n");
-		fd1 = open("testfiles/TestDir1/tracked-1", 
+		BOX_INFO("Rename over existing tracked file");
+		fd1 = open("testfiles/TestDir1/tracked-1",
 			O_CREAT | O_EXCL | O_WRONLY, 0700);
 		fd2 = open("testfiles/TestDir1/tracked-2",
 			O_CREAT | O_EXCL | O_WRONLY, 0700);
@@ -3079,7 +3079,7 @@ int test_bbackupd()
 				== 0);
 		#endif
 
-		TEST_THAT(::rename("testfiles/TestDir1/tracked-1", 
+		TEST_THAT(::rename("testfiles/TestDir1/tracked-1",
 			"testfiles/TestDir1/tracked-2") == 0);
 		TEST_THAT(!TestFileExists("testfiles/TestDir1/tracked-1"));
 		TEST_THAT( TestFileExists("testfiles/TestDir1/tracked-2"));
@@ -3095,7 +3095,7 @@ int test_bbackupd()
 		if (!ServerIsAlive(bbstored_pid)) return 1;
 		if (failures) return 1;
 
-		// case which went wrong: rename a tracked file 
+		// case which went wrong: rename a tracked file
 		// over a deleted file
 		printf("\n==== Rename an existing file over a deleted file\n");
 		TEST_THAT(!TestFileExists("testfiles/TestDir1/x1/dsfdsfs98.fd"));
@@ -3105,7 +3105,7 @@ int test_bbackupd()
 		wait_for_backup_operation("bbackupd to sync");
 
 		TEST_COMPARE(Compare_Same);
-		
+
 		// Check that no read error has been reported yet
 		TEST_THAT(!TestFileExists("testfiles/notifyran.read-error.1"));
 
@@ -3123,7 +3123,7 @@ int test_bbackupd()
 			"testfiles/TestDir1/df9834.dsf") == 0);
 		
 		// Add some more files
-		// Because the 'm' option is not used, these files will 
+		// Because the 'm' option is not used, these files will
 		// look very old to the daemon.
 		// Lucky it'll upload them then!
 		TEST_THAT(unpack_files("test2"));
@@ -3227,7 +3227,7 @@ int test_bbackupd()
 				connect_and_login(context,
 				BackupProtocolLogin::Flags_ReadOnly);
 			
-			std::auto_ptr<BackupStoreDirectory> dir = 
+			std::auto_ptr<BackupStoreDirectory> dir =
 				ReadDirectory(*client);
 
 			int64_t testDirId = SearchDir(*dir, "Test1");
@@ -3268,8 +3268,8 @@ int test_bbackupd()
 			TEST_THAT(!TestFileExists("testfiles/notifyran.read-error.1"));
 
 			// Check that read errors are reported neatly
-			printf("\n==== Add unreadable files\n");
-			
+			BOX_INFO("Add unreadable files");
+
 			{
 				// Dir and file which can't be read
 				TEST_THAT(::mkdir("testfiles/TestDir1/sub23"
@@ -3320,7 +3320,8 @@ int test_bbackupd()
 		::safe_sleep(1);
 
 		{
-			// Open a file, then save something to it every second
+			BOX_INFO("Open a file, then save something to it "
+				"every second for 12 seconds");
 			for(int l = 0; l < 12; ++l)
 			{
 				FILE *f = ::fopen("testfiles/TestDir1/continousupdate", "w+");
@@ -3328,13 +3329,8 @@ int test_bbackupd()
 				fprintf(f, "Loop iteration %d\n", l);
 				fflush(f);
 				fclose(f);
-
-				printf(".");
-				fflush(stdout);
 				safe_sleep(1);
 			}
-			printf("\n");
-			fflush(stdout);
 			
 			// Check there's a difference
 			compareReturnValue = ::system("perl testfiles/"
@@ -3343,8 +3339,8 @@ int test_bbackupd()
 			TEST_RETURN(compareReturnValue, 1);
 			TestRemoteProcessMemLeaks("bbackupquery.memleaks");
 
-			printf("\n==== Keep on continuously updating file, "
-				"check it is uploaded eventually\n");
+			BOX_INFO("Keep on continuously updating file for "
+				"28 seconds, check it is uploaded eventually");
 
 			for(int l = 0; l < 28; ++l)
 			{
@@ -3354,13 +3350,8 @@ int test_bbackupd()
 				fprintf(f, "Loop 2 iteration %d\n", l);
 				fflush(f);
 				fclose(f);
-
-				printf(".");
-				fflush(stdout);
 				safe_sleep(1);
 			}
-			printf("\n");
-			fflush(stdout);
 
 			compareReturnValue = ::system("perl testfiles/"
 				"extcheck2.pl");
@@ -3398,12 +3389,12 @@ int test_bbackupd()
 					BackupProtocolLogin::Flags_ReadOnly);
 
 			// Find the ID of the Test1 directory
-			restoredirid = GetDirID(*client, "Test1", 
+			restoredirid = GetDirID(*client, "Test1",
 				BackupProtocolListDirectory::RootDirectory);
 			TEST_THAT(restoredirid != 0);
 
 			// Test the restoration
-			TEST_THAT(BackupClientRestore(*client, restoredirid, 
+			TEST_THAT(BackupClientRestore(*client, restoredirid,
 				"Test1" /* remote */,
 				"testfiles/restore-Test1" /* local */,
 				true /* print progress dots */,
@@ -3417,24 +3408,24 @@ int test_bbackupd()
 			// to the server, so we'll compare later.
 
 			// Make sure you can't restore a restored directory
-			TEST_THAT(BackupClientRestore(*client, restoredirid, 
-				"Test1", "testfiles/restore-Test1", 
+			TEST_THAT(BackupClientRestore(*client, restoredirid,
+				"Test1", "testfiles/restore-Test1",
 				true /* print progress dots */,
 				false /* restore deleted */,
 				false /* undelete after */,
 				false /* resume */,
 				false /* keep going */) 
 				== Restore_TargetExists);
-			
+
 			// Find ID of the deleted directory
 			deldirid = GetDirID(*client, "x1", restoredirid);
 			TEST_THAT(deldirid != 0);
 
-			// Just check it doesn't bomb out -- will check this 
+			// Just check it doesn't bomb out -- will check this
 			// properly later (when bbackupd is stopped)
-			TEST_THAT(BackupClientRestore(*client, deldirid, 
+			TEST_THAT(BackupClientRestore(*client, deldirid,
 				"Test1", "testfiles/restore-Test1-x1",
-				true /* print progress dots */, 
+				true /* print progress dots */,
 				true /* restore deleted */,
 				false /* undelete after */,
 				false /* resume */,
@@ -3695,7 +3686,7 @@ int test_bbackupd()
 			do_interrupted_restore(context, restoredirid);
 			int64_t resumesize = 0;
 			TEST_THAT(FileExists("testfiles/"
-				"restore-interrupt.boxbackupresume", 
+				"restore-interrupt.boxbackupresume",
 				&resumesize));
 			// make sure it has recorded something to resume
 			TEST_THAT(resumesize > 16);	
@@ -3708,8 +3699,8 @@ int test_bbackupd()
 
 			// Check that the restore fn returns resume possible,
 			// rather than doing anything
-			TEST_THAT(BackupClientRestore(*client, restoredirid, 
-				"Test1", "testfiles/restore-interrupt", 
+			TEST_THAT(BackupClientRestore(*client, restoredirid,
+				"Test1", "testfiles/restore-interrupt",
 				true /* print progress dots */, 
 				false /* restore deleted */, 
 				false /* undelete after */, 
@@ -3718,8 +3709,8 @@ int test_bbackupd()
 				== Restore_ResumePossible);
 
 			// Then resume it
-			TEST_THAT(BackupClientRestore(*client, restoredirid, 
-				"Test1", "testfiles/restore-interrupt", 
+			TEST_THAT(BackupClientRestore(*client, restoredirid,
+				"Test1", "testfiles/restore-interrupt",
 				true /* print progress dots */, 
 				false /* restore deleted */, 
 				false /* undelete after */, 
@@ -3748,8 +3739,8 @@ int test_bbackupd()
 				connect_and_login(context, 0 /* read-write */);
 
 			// Do restore and undelete
-			TEST_THAT(BackupClientRestore(*client, deldirid, 
-				"Test1", "testfiles/restore-Test1-x1-2", 
+			TEST_THAT(BackupClientRestore(*client, deldirid,
+				"Test1", "testfiles/restore-Test1-x1-2",
 				true /* print progress dots */, 
 				true /* deleted files */, 
 				true /* undelete after */,