Tracker: Throw exception on assert failure

... in situations where a NULL pointer dereference was vital to the
functioning of the method we use a stronger assert that throws an
exception on failure. This is accomplished by a new method in
Utilities.cpp, ThrowOnAssert().

None of these conditions should ever be true, if they are it means that
the code is written improperly and would have resulted in a NULL
dereference and undefined behavior (most likely a crash) before.

Most instances of ThrowOnAssert() either replace an ASSERT followed
by a dereference or an early return that covered the error.

Also remove _ThrowOnErrorWithMessage() which wasn't being used.

src/kits/tracker/AttributeStream.cpp

@@ -38,6 +38,8 @@ All rights reserved.
 #include <Debug.h>
 #include <Node.h>
 
+#include "Utilities.h"
+
 
 // ToDo:
 // lazy Rewind from Drive, only if data is available
@@ -309,7 +311,8 @@ AttributeStreamFileNode::SetTo(BNode* node)
 off_t
 AttributeStreamFileNode::Contains(const char* name, uint32 type)
 {
-	ASSERT(fNode);
+	ThrowOnAssert(fNode != NULL);
+
 	attr_info info;
 	if (fNode->GetAttrInfo(name, &info) != B_OK)
 		return 0;
@@ -348,8 +351,7 @@ off_t
 AttributeStreamFileNode::Write(const char* name, const char* foreignName,
 	uint32 type, off_t size, const void* buffer)
 {
-	ASSERT(fNode != NULL);
+	ThrowOnAssert(fNode != NULL);
-	ASSERT(dynamic_cast<BNode*>(fNode) != NULL);
 
 	off_t result = fNode->WriteAttr(name, type, 0, buffer, (size_t)size);
 	if (result == size && foreignName != NULL) {
@@ -365,10 +367,11 @@ AttributeStreamFileNode::Write(const char* name, const char* foreignName,
 bool
 AttributeStreamFileNode::Drive()
 {
-	ASSERT(fNode != NULL);
 	if (!_inherited::Drive())
 		return false;
 
+	ThrowOnAssert(fNode != NULL);
+
 	const AttributeInfo* attr;
 	while ((attr = fReadFrom->Next()) != 0) {
 		const char* data = fReadFrom->Get();
@@ -395,7 +398,7 @@ AttributeStreamFileNode::Get()
 bool
 AttributeStreamFileNode::Fill(char* buffer) const
 {
-	ASSERT(fNode != NULL);
+	ThrowOnAssert(fNode != NULL);
 
 	return fNode->ReadAttr(fCurrentAttr.Name(), fCurrentAttr.Type(), 0,
 		buffer, (size_t)fCurrentAttr.Size()) == (ssize_t)fCurrentAttr.Size();
@@ -405,8 +408,8 @@ AttributeStreamFileNode::Fill(char* buffer) const
 const AttributeInfo*
 AttributeStreamFileNode::Next()
 {
-	ASSERT(fNode != NULL);
 	ASSERT(fReadFrom == NULL);
+	ThrowOnAssert(fNode != NULL);
 
 	char attrName[256];
 	if (fNode->GetNextAttrName(attrName) != B_OK)

src/kits/tracker/ContainerWindow.cpp

@@ -334,8 +334,7 @@ DraggableContainerIcon::MouseDown(BPoint where)
 {
 	// we only like container windows
 	BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());
-	if (window == NULL)
+	ThrowOnAssert(window != NULL);
-		return;
 
 	// we don't like the Trash icon (because it cannot be moved)
 	if (window->IsTrash() || window->IsPrintersDir())
@@ -460,8 +459,7 @@ void
 DraggableContainerIcon::FrameMoved(BPoint)
 {
 	BMenuBar* bar = dynamic_cast<BMenuBar*>(Parent());
-	if (bar == NULL)
+	ThrowOnAssert(bar != NULL);
-		return;
 
 	// TODO: ugly hack following:
 	// This is a trick to get the actual width of all menu items
@@ -492,8 +490,7 @@ void
 DraggableContainerIcon::Draw(BRect updateRect)
 {
 	BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());
-	if (window == NULL)
+	ThrowOnAssert(window != NULL);
-		return;
 
 	if (be_control_look != NULL) {
 		BRect rect(Bounds());

src/kits/tracker/CountView.cpp

@@ -311,8 +311,7 @@ void
 BCountView::MouseDown(BPoint)
 {
 	BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());
-	if (window == NULL)
+	ThrowOnAssert(window != NULL);
-		return;
 
 	window->Activate();
 	window->UpdateIfNeeded();

src/kits/tracker/DesktopPoseView.cpp

@@ -83,7 +83,7 @@ DesktopPoseView::InitDesktopDirentIterator(BPoseView* nodeMonitoringTarget,
 	ASSERT(sourceModel.Node() != NULL);
 
 	BDirectory* sourceDirectory = dynamic_cast<BDirectory*>(sourceModel.Node());
-	ASSERT(sourceDirectory != NULL);
+	ThrowOnAssert(sourceDirectory != NULL);
 
 	// build an iterator list, start with boot
 	EntryListBase* perDesktopIterator
@@ -224,8 +224,7 @@ void
 DesktopPoseView::AdaptToVolumeChange(BMessage* message)
 {
 	TTracker* tracker = dynamic_cast<TTracker*>(be_app);
-	if (tracker == NULL)
+	ThrowOnAssert(tracker != NULL);
-		return;
 
 	bool showDisksIcon = false;
 	bool mountVolumesOnDesktop = true;

src/kits/tracker/FSUtils.cpp

@@ -1243,8 +1243,7 @@ LowLevelCopy(BEntry* srcEntry, StatStruct* srcStat, BDirectory* destDir,
 		char linkpath[MAXPATHLEN];
 
 		ThrowOnError(srcLink.SetTo(srcEntry));
-		ThrowIfNotSize(srcLink.ReadLink(linkpath, MAXPATHLEN-1));
+		ThrowOnError(srcLink.ReadLink(linkpath, MAXPATHLEN - 1));
-
 		ThrowOnError(destDir->CreateSymLink(destName, linkpath, &newLink));
 
 		node_ref destNodeRef;
@@ -1762,10 +1761,11 @@ MoveItem(BEntry* entry, BDirectory* destDir, BPoint* loc, uint32 moveMode,
 		return error.fError;
 	} catch (FailWithAlert error) {
 		BString buffer(error.fString);
-		if (error.fName)
+		if (error.fName != NULL)
 			buffer.ReplaceFirst("%name", error.fName);
 		else
 			buffer << error.fString;
+
 		BAlert* alert = new BAlert("", buffer.String(), B_TRANSLATE("OK"),
 			0, 0, B_WIDTH_AS_USUAL, B_WARNING_ALERT);
 		alert->SetFlags(alert->Flags() | B_CLOSE_ON_ESCAPE);

src/kits/tracker/NavMenu.cpp

@@ -718,6 +718,8 @@ BNavMenu::BuildVolumeMenu()
 int
 BNavMenu::CompareFolderNamesFirstOne(const BMenuItem* i1, const BMenuItem* i2)
 {
+	ThrowOnAssert(i1 != NULL && i2 != NULL);
+
 	const ModelMenuItem* item1 = dynamic_cast<const ModelMenuItem*>(i1);
 	const ModelMenuItem* item2 = dynamic_cast<const ModelMenuItem*>(i2);
 
@@ -733,6 +735,8 @@ BNavMenu::CompareFolderNamesFirstOne(const BMenuItem* i1, const BMenuItem* i2)
 int
 BNavMenu::CompareOne(const BMenuItem* i1, const BMenuItem* i2)
 {
+	ThrowOnAssert(i1 != NULL && i2 != NULL);
+
 	return strcasecmp(i1->Label(), i2->Label());
 }
 

src/kits/tracker/OpenWithWindow.cpp

@@ -1560,6 +1560,8 @@ bool
 SearchForSignatureEntryList::CanOpenWithFilter(const Model* appModel,
 	const BMessage* entriesToOpen, const entry_ref* preferredApp)
 {
+	ThrowOnAssert(appModel != NULL);
+
 	if (!appModel->IsExecutable() || !appModel->Node()) {
 		// weed out non-executable
 #if xDEBUG

src/kits/tracker/PoseView.cpp

@@ -745,14 +745,16 @@ BPoseView::SavePoseLocations(BRect* frameIfDesktop)
 	if (!fSavePoseLocations)
 		return;
 
-	ASSERT(TargetModel());
 	ASSERT(Window()->IsLocked());
 
+	Model* targetModel = TargetModel();
+	ThrowOnAssert(targetModel != NULL);
+
 	BVolume volume(TargetModel()->NodeRef()->device);
 	if (volume.InitCheck() != B_OK)
 		return;
 
-	if (!TargetModel()->IsRoot()
+	if (!targetModel->IsRoot()
 		&& (volume.IsReadOnly() || !volume.KnowsAttr())) {
 		// check that we can write out attrs; Root should always work
 		// because it gets saved on the boot disk but the above checks
@@ -770,7 +772,7 @@ BPoseView::SavePoseLocations(BRect* frameIfDesktop)
 			poseInfo.fInvisible = false;
 
 			if (model->IsRoot())
-				poseInfo.fInitedDirectory = TargetModel()->NodeRef()->node;
+				poseInfo.fInitedDirectory = targetModel->NodeRef()->node;
 			else
 				poseInfo.fInitedDirectory = model->EntryRef()->directory;
 
@@ -1331,18 +1333,10 @@ BPoseView::AddPosesTask(void* castToParams)
 	thread_id threadID = find_thread(NULL);
 
 	BPoseView* view = dynamic_cast<BPoseView*>(lock.Handler());
-
+	ThrowOnAssert(view != NULL);
-	ASSERT(view != NULL);
-
-	if (view == NULL)
-		return B_ERROR;
 
 	BWindow* window = dynamic_cast<BWindow*>(lock.Looper());
-
+	ThrowOnAssert(window != NULL);
-	ASSERT(window != NULL);
-
-	if (window == NULL)
-		return B_ERROR;
 
 	// allocate the iterator we will use for adding poses; this
 	// can be a directory or any other collection of entry_refs, such
@@ -3392,7 +3386,7 @@ void
 BPoseView::NewFileFromTemplate(const BMessage* message)
 {
 	Model* targetModel = TargetModel();
-	ASSERT(targetModel != NULL);
+	ThrowOnAssert(targetModel != NULL);
 
 	entry_ref destEntryRef;
 	node_ref destNodeRef;
@@ -3413,7 +3407,7 @@ BPoseView::NewFileFromTemplate(const BMessage* message)
 
 	if (dir.InitCheck() == B_OK) {
 		// special handling of directories
-		if (FSCreateNewFolderIn(TargetModel()->NodeRef(), &destEntryRef,
+		if (FSCreateNewFolderIn(targetModel->NodeRef(), &destEntryRef,
 				&destNodeRef) == B_OK) {
 			BEntry destEntry(&destEntryRef);
 			destEntry.Rename(fileName);
@@ -3452,7 +3446,7 @@ BPoseView::NewFileFromTemplate(const BMessage* message)
 
 	// start renaming the entry
 	int32 index;
-	BPose* pose = EntryCreated(TargetModel()->NodeRef(), &destNodeRef,
+	BPose* pose = EntryCreated(targetModel->NodeRef(), &destNodeRef,
 		destEntryRef.name, &index);
 
 	if (fFiltering) {
@@ -3477,18 +3471,19 @@ BPoseView::NewFileFromTemplate(const BMessage* message)
 void
 BPoseView::NewFolder(const BMessage* message)
 {
-	ASSERT(TargetModel());
+	Model* targetModel = TargetModel();
+	ThrowOnAssert(targetModel != NULL);
 
 	entry_ref ref;
 	node_ref nodeRef;
 
-	if (FSCreateNewFolderIn(TargetModel()->NodeRef(), &ref, &nodeRef) == B_OK) {
+	if (FSCreateNewFolderIn(targetModel->NodeRef(), &ref, &nodeRef) == B_OK) {
 		// try to place new folder at click point or under mouse if possible
 
 		PlaceFolder(&ref, message);
 
 		int32 index;
-		BPose* pose = EntryCreated(TargetModel()->NodeRef(), &nodeRef, ref.name,
+		BPose* pose = EntryCreated(targetModel->NodeRef(), &nodeRef, ref.name,
 			&index);
 
 		if (fFiltering) {
@@ -4888,10 +4883,12 @@ static bool
 AddOneToLaunchMessage(BPose* pose, BPoseView*, void* castToParams)
 {
 	LaunchParams* params = (LaunchParams*)castToParams;
+	ThrowOnAssert(params != NULL);
+	ThrowOnAssert(pose != NULL);
+	ThrowOnAssert(pose->TargetModel() != NULL);
 
-	ASSERT(pose->TargetModel());
 	if (params->app->IsDropTarget(params->checkTypes
-			? pose->TargetModel() : 0, true)) {
+			? pose->TargetModel() : NULL, true)) {
 		params->refsMessage->AddRef("refs", pose->TargetModel()->EntryRef());
 	}
 
@@ -5637,7 +5634,7 @@ BPoseView::EntryMoved(const BMessage* message)
 	}
 
 	Model* targetModel = TargetModel();
-	ASSERT(targetModel != NULL);
+	ThrowOnAssert(targetModel != NULL);
 
 	node_ref thisDirNode;
 	if (ContainerWindow()->IsTrash()) {
@@ -6097,9 +6094,9 @@ BPoseView::MoveListToTrash(BObjectList<entry_ref>* list, bool selectNext,
 		pointInPose.y += fListElemHeight * index;
 
 		TTracker* tracker = dynamic_cast<TTracker*>(be_app);
+		if (tracker != NULL) {
+			ThrowOnAssert(TargetModel() != NULL);
 
-		ASSERT(TargetModel());
-		if (tracker) {
 			// add a function object to the list of tasks to run
 			// that will select the next item after the one we just
 			// deleted
@@ -6320,6 +6317,8 @@ BPoseView::Delete(BObjectList<entry_ref>* list, bool selectNext, bool askUser)
 
 		TTracker* tracker = dynamic_cast<TTracker*>(be_app);
 		if (tracker != NULL) {
+			ThrowOnAssert(TargetModel() != NULL);
+
 			// add a function object to the list of tasks to run
 			// that will select the next item after the one we just
 			// deleted
@@ -6361,6 +6360,8 @@ BPoseView::RestoreItemsFromTrash(BObjectList<entry_ref>* list, bool selectNext)
 
 		TTracker* tracker = dynamic_cast<TTracker*>(be_app);
 		if (tracker != NULL) {
+			ThrowOnAssert(TargetModel() != NULL);
+
 			// add a function object to the list of tasks to run
 			// that will select the next item after the one we just
 			// restored

src/kits/tracker/QueryPoseView.cpp

@@ -353,7 +353,8 @@ BQueryPoseView::InitDirentIterator(const entry_ref* ref)
 		delta *=  1000000;
 
 		TTracker* tracker = dynamic_cast<TTracker*>(be_app);
-		ASSERT(tracker);
+		ThrowOnAssert(tracker != NULL);
+
 		tracker->MainTaskLoop()->RunLater(
 			NewLockingMemberFunctionObject(&BQueryPoseView::Refresh, this),
 			delta);

src/kits/tracker/TextWidget.cpp

@@ -297,8 +297,14 @@ TextViewFilter(BMessage* message, BHandler**, BMessageFilter* filter)
 	if (message->FindInt8("byte", (int8*)&key) != B_OK)
 		return B_DISPATCH_MESSAGE;
 
-	BPoseView* poseView = dynamic_cast<BContainerWindow*>(
+	ThrowOnAssert(filter != NULL);
-		filter->Looper())->PoseView();
+
+	BContainerWindow* window = dynamic_cast<BContainerWindow*>(
+		filter->Looper());
+	ThrowOnAssert(window != NULL);
+
+	BPoseView* poseView = window->PoseView();
+	ThrowOnAssert(poseView != NULL);
 
 	if (key == B_RETURN || key == B_ESCAPE) {
 		poseView->CommitActivePose(key == B_RETURN);

src/kits/tracker/Utilities.cpp

@@ -1654,12 +1654,15 @@ ComputeTypeAheadScore(const char* text, const char* match, bool wordMode)
 }
 
 
+//	#pragma mark - throw on error functions.
+
+
 void
 _ThrowOnError(status_t result, const char* DEBUG_ONLY(file),
 	int32 DEBUG_ONLY(line))
 {
 	if (result != B_OK) {
-		PRINT(("failing %s at %s:%d\n", strerror(result), file, (int)line));
+		PRINT(("%s at %s:%d\n", strerror(result), file, (int)line));
 		throw result;
 	}
 }
@@ -1670,20 +1673,19 @@ _ThrowIfNotSize(ssize_t size, const char* DEBUG_ONLY(file),
 	int32 DEBUG_ONLY(line))
 {
 	if (size < B_OK) {
-		PRINT(("failing %s at %s:%d\n", strerror(size), file, (int)line));
+		PRINT(("%s at %s:%d\n", strerror((status_t)size), file, (int)line));
 		throw (status_t)size;
 	}
 }
 
 
 void
-_ThrowOnError(status_t result, const char* DEBUG_ONLY(debugString),
+_ThrowOnAssert(bool success, const char* DEBUG_ONLY(file),
-	const char* DEBUG_ONLY(file), int32 DEBUG_ONLY(line))
+	int32 DEBUG_ONLY(line))
 {
-	if (result != B_OK) {
+	if (!success) {
-		PRINT(("failing %s, %s at %s:%d\n", debugString, strerror(result), file,
+		PRINT(("Assert failed at %s:%d\n", file, (int)line));
-			(int)line));
+		throw B_ERROR;
-		throw result;
 	}
 }
 

src/kits/tracker/Utilities.h

@@ -474,20 +474,18 @@ ThrowOnInitCheckError(InitCheckable* item)
 }
 
 #if DEBUG
-#	define ThrowOnError(result) _ThrowOnError(result, __FILE__, __LINE__)
+#	define ThrowOnError(x) _ThrowOnError(x, __FILE__, __LINE__)
-#	define ThrowIfNotSize(result) _ThrowIfNotSize(result, __FILE__, __LINE__)
+#	define ThrowIfNotSize(x) _ThrowIfNotSize(x, __FILE__, __LINE__)
-#	define ThrowOnErrorWithMessage(result, debugStr) \
+#	define ThrowOnAssert(x) _ThrowOnAssert(x, __FILE__, __LINE__)
-		_ThrowOnError(result, debugStr, __FILE__, __LINE__)
 #else
-#	define ThrowOnError(x) _ThrowOnError(x, 0, 0)
+#	define ThrowOnError(x) _ThrowOnError(x, NULL, 0)
-#	define ThrowIfNotSize(x) _ThrowIfNotSize(x, 0, 0)
+#	define ThrowIfNotSize(x) _ThrowIfNotSize(x, NULL, 0)
-#	define ThrowOnErrorWithMessage(result, debugStr) \
+#	define ThrowOnAssert(x) _ThrowOnAssert(x, NULL, 0)
-		_ThrowOnError(result, debugStr, __FILE__, __LINE__)
 #endif
 
 void _ThrowOnError(status_t, const char*, int32);
 void _ThrowIfNotSize(ssize_t, const char*, int32);
-void _ThrowOnError(status_t, const char* debugStr, const char*, int32);
+void _ThrowOnAssert(bool, const char*, int32);
 
 // stub calls that work around BAppFile info inefficiency
 status_t GetAppSignatureFromAttr(BFile*, char*);

src/kits/tracker/WidgetAttributeText.cpp

@@ -556,8 +556,7 @@ int
 StringAttributeText::Compare(WidgetAttributeText& attr, BPoseView* view)
 {
 	StringAttributeText* compareTo = dynamic_cast<StringAttributeText*>(&attr);
-
+	ThrowOnAssert(compareTo != NULL);
-	ASSERT(compareTo != NULL);
 
 	if (fValueDirty)
 		ReadValue(&fFullValueText);
@@ -647,8 +646,7 @@ int
 ScalarAttributeText::Compare(WidgetAttributeText& attr, BPoseView*)
 {
 	ScalarAttributeText* compareTo = dynamic_cast<ScalarAttributeText*>(&attr);
-
+	ThrowOnAssert(compareTo != NULL);
-	ASSERT(compareTo != NULL);
 
 	if (fValueDirty)
 		fValue = ReadValue();
@@ -756,8 +754,7 @@ int
 NameAttributeText::Compare(WidgetAttributeText& attr, BPoseView* view)
 {
 	NameAttributeText* compareTo = dynamic_cast<NameAttributeText*>(&attr);
-
+	ThrowOnAssert(compareTo != NULL);
-	ASSERT(compareTo != NULL);
 
 	if (fValueDirty)
 		ReadValue(&fFullValueText);
@@ -892,8 +889,7 @@ RealNameAttributeText::Compare(WidgetAttributeText& attr, BPoseView* view)
 {
 	RealNameAttributeText* compareTo
 		= dynamic_cast<RealNameAttributeText*>(&attr);
-
+	ThrowOnAssert(compareTo != NULL);
-	ASSERT(compareTo != NULL);
 
 	if (fValueDirty)
 		ReadValue(&fFullValueText);
@@ -1567,8 +1563,7 @@ GenericAttributeText::Compare(WidgetAttributeText& attr, BPoseView*)
 {
 	GenericAttributeText* compareTo
 		= dynamic_cast<GenericAttributeText*>(&attr);
-
+	ThrowOnAssert(compareTo != NULL);
-	ASSERT(compareTo != NULL);
 
 	if (fValueDirty)
 		ReadValue(&fFullValueText);